Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: rename API support overwrite changes to files #898

Merged
merged 9 commits into from
Nov 28, 2023

Conversation

amyXia1994
Copy link
Contributor

1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):

  • N
  • Y

re #880

2. What is the scope of this PR (e.g. component or file name):

3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):

feat: rename API support overwrite changes to files

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Other

4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):

  • N
  • Y

5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:

  • Unit test
  • Integration test
  • Benchmark (add benchmark stats below)
  • Manual test (add detailed scripts or steps below)
  • Other

@coveralls
Copy link
Collaborator

coveralls commented Nov 20, 2023

Pull Request Test Coverage Report for Build 7013602710

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

  • 345 of 355 (97.18%) changed or added relevant lines in 10 files are covered.
  • 241 unchanged lines in 8 files lost coverage.
  • Overall coverage decreased (-15.2%) to 72.588%

Changes Missing Coverage Covered Lines Changed/Added Lines %
kclvm/tools/src/LSP/src/find_refs.rs 1 2 50.0%
kclvm/tools/src/LSP/src/request.rs 5 6 83.33%
kclvm/tools/src/LSP/src/util.rs 9 10 90.0%
kclvm/tools/src/LSP/src/rename.rs 266 273 97.44%
Files with Coverage Reduction New Missed Lines %
compiler_base/error/src/diagnostic/diagnostic_message.rs 3 0.0%
compiler_base/error/src/diagnostic/components.rs 6 0.0%
compiler_base/error/src/errors.rs 8 0.0%
compiler_base/error/src/diagnostic/diagnostic_handler.rs 11 0.0%
compiler_base/session/src/lib.rs 26 58.06%
compiler_base/error/src/diagnostic/mod.rs 27 0.0%
compiler_base/error/src/diagnostic/style.rs 41 0.0%
compiler_base/error/src/emitter.rs 119 0.0%
Totals Coverage Status
Change from base Build 6967469271: -15.2%
Covered Lines: 42146
Relevant Lines: 58062

💛 - Coveralls

Copy link
Contributor

@Peefy Peefy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Peefy Peefy added api Issues or PRs related to kcl rust native APIs and multi-lang APIs tool Issues or PRs related to kcl tools inlucding format, lint, validation, document tools, etc. labels Nov 22, 2023
@Peefy Peefy added this to the v0.7.0 Release milestone Nov 22, 2023
kclvm/spec/gpyrpc/gpyrpc.proto Outdated Show resolved Hide resolved
kclvm/api/src/service/service_impl.rs Outdated Show resolved Hide resolved
kclvm/api/src/service/service_impl.rs Outdated Show resolved Hide resolved
Signed-off-by: xiarui.xr <[email protected]>
Signed-off-by: xiarui.xr <[email protected]>
Signed-off-by: xiarui.xr <[email protected]>
@Peefy
Copy link
Contributor

Peefy commented Nov 27, 2023

@CodiumAI-Agent /review

@CodiumAI-Agent
Copy link

PR Analysis

  • 🎯 Main theme: Adding a feature to rename symbols in files and apply changes to the files
  • 📝 PR summary: This PR introduces a new feature to the KCL language server that allows renaming symbols in files. It provides the ability to find all occurrences of a symbol in a set of files and rename them to a new name. The changes are then applied to the files. The PR also includes updates to the tests to cover the new functionality.
  • 📌 Type of PR: Enhancement
  • 🧪 Relevant tests added: Yes
  • ⏱️ Estimated effort to review [1-5]: 4, because the PR introduces a new feature with a significant amount of new code. It requires a good understanding of the existing codebase and the language server protocol to review effectively.

PR Feedback

  • 💡 General suggestions: The PR is well-structured and the new feature is implemented in a modular way. The code is readable and follows good coding practices. However, it would be beneficial to add more comments to the new functions to explain their purpose and functionality in more detail. This would make it easier for other developers to understand the code.

  • 🤖 Code feedback:
    • relevant file: kclvm/tools/src/LSP/src/rename.rs
      suggestion: Consider handling the case where the file cannot be read in the 'apply_rename_changes' function. Currently, the function uses unwrap which may cause the program to panic if the file cannot be read. Instead, you could return an error to the caller. [important]
      relevant line: '+ let file_content = read_file(

How to use

Instructions

Tag me in a comment '@CodiumAI-Agent' and add one of the following commands:
/review: Request a review of your Pull Request.
/describe: Update the PR title and description based on the contents of the PR.
/improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback.
/ask <QUESTION>: Ask a question about the PR.
/update_changelog: Update the changelog based on the PR's contents.
/add_docs: Generate docstring for new components introduced in the PR.
/generate_labels: Generate labels for the PR based on the PR's contents.
see the tools guide for more details.

To edit any configuration parameter from the configuration.toml, add --config_path=new_value.
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, add a /config comment.

@Peefy Peefy changed the title feat: rename API support overwrite changes to files feat: rename API and rename code API support overwrite changes to files Nov 27, 2023
@Peefy Peefy changed the title feat: rename API and rename code API support overwrite changes to files feat: rename API support overwrite changes to files Nov 27, 2023
@Peefy Peefy merged commit 61e8221 into kcl-lang:main Nov 28, 2023
10 of 11 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api Issues or PRs related to kcl rust native APIs and multi-lang APIs tool Issues or PRs related to kcl tools inlucding format, lint, validation, document tools, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants