This repository was archived by the owner on Aug 30, 2025. It is now read-only.
WIP add support for renaming function and macro#720
Draft
scottming wants to merge 5 commits into
Draft
Conversation
Enhanced the application by integrating rename functionality, increasing its editing capabilities. This includes supporting the preparation and execution of rename operations in various modules such as the lexical AST module, protocol request and response modules, remote control API, and server provider handling. Key additions entail aliasing for easier reference, handling new rename-related requests and responses, and implementing server state updates to acknowledge rename capabilities. The changes underscore our dedication to improving the tool's utility and ensuring a more dynamic and responsive user experience in code manipulation scenarios. Filter out the `:struct` type references Add some handler tests for `rename` Add doc for `local_module_name` Move `rename` from `code_intelligence` to `code_mod` Move the prepare logic to a individual module This PR is primarily copied from #374, but compared to the previous implementation, we have made two changes: 1. We constrained the scope of renaming, meaning that renaming a module is only allowed at its definition. This simplification reduces a lot of computations. 2. Based on the first point, our preparation no longer returns just a single local module, but instead, the entire module. This means that module renaming has become more powerful. We can not only rename a single local module but also simplify a module. For example, renaming TopLevel.Parent.Child to TopLevel.Child, or renaming some middle parts, like TopLevel.Parent.Child to TopLevel.Renamed. I personally really like this change, especially the second point, which makes module renaming much more practical. Surround the whole module when renaming happens Remove logic related to the rename function. Apply some code review suggestions Fix a bug when expanding the module Fix a merge error Apply a format module usage suggestion Update apps/remote_control/lib/lexical/remote_control/code_mod/rename.ex Co-authored-by: Steve Cohen <scohen@scohen.org> Apply the `defdelegate` suggestion Fix a bug when the descendant containing the old suffix
* Rename the file while renaming the module. It first determines whether the module meets certain hard constraints, such as having no parent, then performs some conventional checks. If both are satisfied, it proceeds to return the file renaming. * Maybe insert special folder for PhoenixWeb's module * Apply some code review changes for using `Document.Changes` Use `Document.Changes` instead of `CodeMod.Rename.DocumentChanges` Make `Document.Changes.RenameFile` a struct and use `root_module?` instead of `has_silibings?` and `has_parent?` * Use `rename progress marker` instead of suspending build * Use `file_changed` instead of `file_compile_requested` for reindexing * `uri_with_operations_counts` by client name * Apply some code review suggestions for `rename/file` * Apply suggestions from code review Co-authored-by: Steve Cohen <scohen@scohen.org> * Do not need to care about the `DidClose` message * Apply some code review suggestions 1. Move the `if-else` logic to the `Commands.Rename` module 2. Add `file_opened` message type 3. modify the format error message * Shouldn't returns `RenameFile` if the file name not changed * Change `uri_with_operation_counts` to `uri_with_expected_operation` and special some message type for emacs --------- Co-authored-by: Steve Cohen <scohen@scohen.org>
3df5e6c to
03c5889
Compare
scohen
reviewed
Apr 28, 2024
Comment on lines
+8
to
+11
| alias Lexical.Document.Range | ||
|
|
||
| alias Lexical.RemoteControl.CodeIntelligence.Entity |
Collaborator
There was a problem hiding this comment.
Suggested change
| alias Lexical.Document.Range | |
| alias Lexical.RemoteControl.CodeIntelligence.Entity | |
| alias Lexical.Document.Range | |
| alias Lexical.RemoteControl.CodeIntelligence.Entity |
|
|
||
| with {:ok, _} <- Document.Store.open_temporary(uri), | ||
| {:ok, _document, analysis} <- Document.Store.fetch(uri, :analysis) do | ||
| Ast.Callable.fetch_name_range(analysis, entry.range.start, local_name) |
Collaborator
There was a problem hiding this comment.
Suggested change
| Ast.Callable.fetch_name_range(analysis, entry.range.start, local_name) | |
| Callable.fetch_name_range(analysis, entry.range.start, local_name) |
|
|
||
| with {:ok, _} <- Document.Store.open_temporary(uri), | ||
| {:ok, _document, analysis} <- Document.Store.fetch(uri, :analysis) do | ||
| Ast.Callable.fetch_name_range(analysis, entry.range.start, local_name) |
Collaborator
There was a problem hiding this comment.
Would it make more sense to use Sourceror's patch functionality here, then use Ast.patch_to_edit to turn them into edits?
There's already Sourceror.Patch.rename_call
| defp name_range({callable_name, meta, _params_and_body}, position) | ||
| when is_atom(callable_name) do | ||
| callable_name = to_string(callable_name) | ||
| start_position = %{position | character: meta[:column]} |
Collaborator
There was a problem hiding this comment.
Suggested change
| start_position = %{position | character: meta[:column]} | |
| start_position = %Position{position | character: start_column} |
|
|
||
| defp name_range({callable_name, meta, _params_and_body}, position) | ||
| when is_atom(callable_name) do | ||
| callable_name = to_string(callable_name) |
Collaborator
There was a problem hiding this comment.
start_column = meta[:column]
| when is_atom(callable_name) do | ||
| callable_name = to_string(callable_name) | ||
| start_position = %{position | character: meta[:column]} | ||
| end_position = %{position | character: meta[:column] + String.length(callable_name)} |
Collaborator
There was a problem hiding this comment.
Suggested change
| end_position = %{position | character: meta[:column] + String.length(callable_name)} | |
| end_position = %{position | character: start_column + String.length(callable_name)} |
Comment on lines
+44
to
+45
| defp name_range({{:., meta, [_aliases, callable_name]}, _, _}, position) do | ||
| dot_length = 1 |
Collaborator
There was a problem hiding this comment.
Suggested change
| defp name_range({{:., meta, [_aliases, callable_name]}, _, _}, position) do | |
| dot_length = 1 | |
| @dot_length 1 | |
| defp name_range({{:., meta, [_aliases, callable_name]}, _, _}, position) do |
| @@ -0,0 +1,62 @@ | |||
| defmodule Lexical.Ast.Callable do | |||
Collaborator
There was a problem hiding this comment.
you said there were problems with indexing these things, why not fix them in the indexer rather than here?
Comment on lines
+687
to
+688
| This is a bug in our function indexation: | ||
| when the cursor is at the reference for a imported call, the definition function can't be found. |
Collaborator
Author
There was a problem hiding this comment.
Still wip, so I use test record the issue first
0806c22 to
cd3d0e1
Compare
Still WIP, only finished the function part. Add a failed test
cd3d0e1 to
1ec5b81
Compare
6bdac06 to
fc4eb47
Compare
fc4eb47 to
3262567
Compare
b5229ff to
8b40671
Compare
55dafa2 to
5f2a602
Compare
11c8ac7 to
2ffe206
Compare
96d3ffb to
d72a46f
Compare
e4bb866 to
2898e78
Compare
2898e78 to
a21b40c
Compare
7862fb5 to
528a1c0
Compare
528a1c0 to
4b2ae48
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Still WIP, only finished the function part.
CleanShot.2024-04-28.at.16.40.24.mp4