Rename file when renaming module #651

scottming · 2024-03-19T14:09:17Z

This PR is an implementation of renaming 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.

CleanShot.2024-03-20.at.21.29.46.mp4

scohen · 2024-03-19T20:19:47Z

In the end, don't we just want the module name to match the file path? I was thinking this could be accomplished by indexing the file with the Module extractor, then, for each module, getting its name, and if it's not in the right file, doing one of the following:

If there is one module in the file:

If the file has the correct name, then do nothing
If the file name or path is incorrect, determine the right path and issue a RenameFIle, then emit edits for the file with the new contents.

If there is more than one module in the file:

Determine if any of the modules have the correct name
If one does, emit an edit for the current file with the others removed
For all modules in the current file that don't have the right name, follow the above steps.

There's a caveat here, since there can be global aliases / imports, etc. in the file, and if there are, they need to be relocated to the new files as well.

apps/remote_control/lib/lexical/remote_control/code_mod/rename/file.ex

scottming · 2024-03-20T13:21:13Z

In the end, don't we just want the module name to match the file path?

Yes, essentially, that's exactly what we need to do.

I was thinking this could be accomplished by indexing the file with the Module extractor

If we can achieve that, it would certainly be ideal, but these steps also sound quite challenging.

scohen · 2024-03-20T20:56:26Z

Why do you think it's challenging?

scottming · 2024-03-21T03:46:21Z

Why do you think it's challenging?

Perhaps I was overthinking it at the time, feeling that there were too many conventions. However, if we start with some simple conventions, like in this PR, then the complexity should be manageable.

apps/remote_control/lib/lexical/remote_control/code_mod/rename/file.ex

scohen · 2024-03-20T21:03:03Z

apps/remote_control/lib/lexical/remote_control/code_mod/rename/file.ex

+  alias Lexical.RemoteControl
+  alias Lexical.RemoteControl.Search.Store
+
+  def maybe_rename(entry, new_suffix) do


maybe just pass the project in here rather than using RemoteControl.get_project(). It makes it easier to test.

That means I need to pass the project down from RemoteControl.Api.rename all the way through. I'm not very keen on doing that; it doesn't seem to simplify the testing. prefer to your above suggestion.

it would simplify testing because you don't need to set something that's effectively global.

apps/remote_control/lib/lexical/remote_control/code_mod/rename/document_changes.ex

scohen · 2024-03-21T17:23:07Z

Why do we care if there are parents or siblings before we rename a file?

scottming · 2024-03-22T12:40:51Z

Why do we care if there are parents or siblings before we rename a file?

When there are some sibling modules at the first level, we usually don't use one of those modules as a mapping for the file name.

Regarding the issue of parent modules, consider examples like state modules inside GenServer modules.

There are tests for these cases.

scohen · 2024-03-25T20:25:00Z

Regarding the issue of parent modules, consider examples like state modules inside GenServer modules.

This is why I was suggesting using the indexer, you can index the file with the module extractor, and see if there are multiple modules in the file (as well as get their hierarchy). If there are, you would treat the file as the parent module, and only rename that.

There are multiple cases for this, of course. You can have a file whose name has no bearing on the modules contained inside as well.

scottming · 2024-03-27T07:20:10Z

This is why I was suggesting using the indexer, you can index the file with the module extractor, and see if there are multiple modules in the file (as well as get their hierarchy)

To be honest, I'm not sure exactly how to do it. It sounds like I need to add some fields to Indexer.Entry, right? And append this information when Entry.block_definition is called. Do you want me to take care of this in this PR?

scohen · 2024-03-27T20:32:40Z

It sounds like I need to add some fields to Indexer.Entry, right?

No, I don't think so. You just need to run the indexer on the document, giving it only the module extractor, and it will return all the module definitions in the document.
You can then look at the entries to find out if they're nested (you can get all the top-level modules by collecting those modules whose block_id is :root. any others are nested.

to index using a subset of extractors, do the following

document
modules =
  |> Indexer.Source.index([Indexer.Extractors.Module])
  |> Enum.reject(& &1.type == :metadata)

Co-authored-by: Steve Cohen <[email protected]>

1. Move the `if-else` logic to the `Commands.Rename` module 2. Add `file_opened` message type 3. modify the format error message

and special some message type for emacs

* 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 <[email protected]> * 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 <[email protected]>

* 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 <[email protected]> * 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 <[email protected]> Add `WorkDoneProgress` for the rename expected_operation Apply suggestions from code review Co-authored-by: Steve Cohen <[email protected]> Update apps/remote_control/lib/lexical/remote_control/code_mod/rename/module.ex Co-authored-by: Steve Cohen <[email protected]> Update apps/remote_control/lib/lexical/remote_control/commands/rename.ex Co-authored-by: Zach Allaun <[email protected]> Use record message instead of Protocol structs Replace the asynchronous subscription method of `dispatch` with synchronous `updates`. In the previous commit, we tried the method of registering and subscribing to messages via `dispatch`. I found that this method has significant issues, as it causes the message triggering compilation, the modification of rename status, and the judgment of progress to be out of sync, leading to rename failures and crashes. Apply another code review suggestions

scottming force-pushed the rename-file-when-renaming-module branch from 257e77c to ca45d9e Compare March 19, 2024 14:10

scohen reviewed Mar 19, 2024

View reviewed changes

scottming force-pushed the rename-file-when-renaming-module branch 5 times, most recently from f097634 to c02d2f6 Compare March 20, 2024 11:51

scottming force-pushed the rename-module-v2 branch from e86d49e to dc10439 Compare March 20, 2024 13:03

scottming force-pushed the rename-file-when-renaming-module branch from c02d2f6 to f6f6972 Compare March 20, 2024 13:18

scottming marked this pull request as ready for review March 20, 2024 13:18

scottming force-pushed the rename-file-when-renaming-module branch from f6f6972 to 49008bc Compare March 20, 2024 13:51

scottming mentioned this pull request Mar 21, 2024

Add Support for Renaming module #636

Open

scottming force-pushed the rename-file-when-renaming-module branch from 10c36b6 to 91d6f58 Compare March 21, 2024 11:14

scohen reviewed Mar 21, 2024

View reviewed changes

scottming force-pushed the rename-module-v2 branch from 5cdad5f to 95cd316 Compare March 27, 2024 09:50

scottming force-pushed the rename-file-when-renaming-module branch from 4d41168 to 44955b4 Compare March 27, 2024 09:57

scottming force-pushed the rename-module-v2 branch from 95cd316 to a1303c8 Compare March 28, 2024 07:37

scottming force-pushed the rename-file-when-renaming-module branch from 44955b4 to 9e4ee39 Compare March 28, 2024 07:46

scottming force-pushed the rename-module-v2 branch from a1303c8 to bc5ae0c Compare March 29, 2024 02:26

scottming force-pushed the rename-file-when-renaming-module branch 2 times, most recently from 2e7c300 to 2e6cb4c Compare March 29, 2024 08:44

scottming and others added 6 commits April 20, 2024 09:17

uri_with_operations_counts by client name

523e5e6

Apply some code review suggestions for rename/file

2ada97d

Apply suggestions from code review

d4ff5b0

Co-authored-by: Steve Cohen <[email protected]>

Do not need to care about the DidClose message

8fa028d

Apply some code review suggestions

78f71e6

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

5e895b9

scottming force-pushed the rename-file-when-renaming-module branch 2 times, most recently from c39424e to c2f5029 Compare April 21, 2024 01:46

scottming mentioned this pull request Apr 21, 2024

Support for renaming modules as well as simultaneously renaming related files. #710

Open

scottming force-pushed the rename-file-when-renaming-module branch from c2f5029 to 35d7caf Compare April 21, 2024 12:18

Change uri_with_operation_counts to uri_with_expected_operation

a36ad78

and special some message type for emacs

scottming force-pushed the rename-file-when-renaming-module branch from 35d7caf to a36ad78 Compare April 21, 2024 12:23

scottming merged commit a971f2a into rename-module-v2 Apr 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename file when renaming module #651

Rename file when renaming module #651

scottming commented Mar 19, 2024 •

edited

Loading

scohen commented Mar 19, 2024

scottming commented Mar 20, 2024

scohen commented Mar 20, 2024

scottming commented Mar 21, 2024

scohen Mar 20, 2024

scottming Mar 22, 2024

scohen Apr 1, 2024

scohen commented Mar 21, 2024

scottming commented Mar 22, 2024 •

edited

Loading

scohen commented Mar 25, 2024

scottming commented Mar 27, 2024

scohen commented Mar 27, 2024

Rename file when renaming module #651

Rename file when renaming module #651

Conversation

scottming commented Mar 19, 2024 • edited Loading

scohen commented Mar 19, 2024

scottming commented Mar 20, 2024

scohen commented Mar 20, 2024

scottming commented Mar 21, 2024

scohen Mar 20, 2024

Choose a reason for hiding this comment

scottming Mar 22, 2024

Choose a reason for hiding this comment

scohen Apr 1, 2024

Choose a reason for hiding this comment

scohen commented Mar 21, 2024

scottming commented Mar 22, 2024 • edited Loading

scohen commented Mar 25, 2024

scottming commented Mar 27, 2024

scohen commented Mar 27, 2024

scottming commented Mar 19, 2024 •

edited

Loading

scottming commented Mar 22, 2024 •

edited

Loading