-
-
Notifications
You must be signed in to change notification settings - Fork 40
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: add alias refactor workspace command #386
feat: add alias refactor workspace command #386
Conversation
test/next_ls/commands/alias_test.exs
Outdated
Bar.to_list(map) | ||
end | ||
|
||
def bar do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why Macro.to_string picks to do/end block the one-liner. The ast from a one liner and a do/end block is the same so that's probably why. I might be missing some context data in one of the cases when testing in iex.
lib/next_ls.ex
Outdated
position = arguments["position"] | ||
text = lsp.assigns.documents[uri] | ||
|
||
NextLS.Commands.Alias.refactor(%{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets just call the "main method" function of these modules "run"
@NJichev is this ready for review? |
2fa58c5
to
36d1030
Compare
It's ready for review(s) now |
So I tried it out and was seeing some exceptions thrown. To use AST with Sourceror, you need to pass the same literal_encoder as we're doing in the pipe command. This is because literals don't contain metadata (as they are literals), so we use the literal encoder to turn them into normal ast nodes that have the metadata. I made that change and now the tests aren't passing, so you'll want to make that change and then get the tests passing again. |
lib/foo.ex
Outdated
@@ -0,0 +1,22 @@ | |||
defmodule Foo do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this file accidentally added?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep my bad
Ready for another review? |
d309f82
to
4e7fc26
Compare
Yeah, it should be good for review |
@NJichev have you tested this in an editor? I can't seem to get it to work at all now. |
ahh, it doesn't work with a remote call with args, like |
You'll need to modify this to work with structs and also inside |
098a792
to
61a25fc
Compare
Those issues should be fixed now |
CleanShot.2024-03-27.at.15.16.57.mp4found another bug. tried to alias this struct and it added the alias to a submodule below and didn't actually change any of the references |
This adds an alias refactor workspace command to NextLS. Your cursor should be at the target module that you wish to alias. It will insert the alias at the of the nearest defmodule definition scoping the refactor only to the current module instead of the whole file.
Rework parts of the code since do/end block is a different ast node, it wasn't matching hence crashing the old code
Also make alias refactor include the start of the module as a viable cursor position. Fix test with bad character ending.
Also refactors the labeling on the workspace commands.
19240d1
to
61b8b66
Compare
i noticed that this happens for the require module code action as well, if you get some time, please fix that in another PR as well. |
I think I already fixed it here, the only work left here is to update spitfire and handle the comments metadata. So the issue was that the find nearest module did literally this, and it found the nearest defmodule instead of the defmodule context of the cursor. Unless it's another issue. I'll take a look during the weekend and finish this PR. |
oh gotcha, cool. i'll check again once we get this merged |
Latest issue (after rebasing on main): it seems to be putting the alias in the right module, but it seems to be duplicating some comments and shifting them into a weird spot CleanShot.2024-04-13.at.09.29.06.mp4 |
The comments were duplicating since we alter the AST only for one module. Filter only to comments within the module.
It fixes the issue with joining newline strings
Glorious! |
This adds an alias refactor workspace command to NextLS.
Your cursor should be at the target module that you wish to alias. It will insert the alias at the top of the nearest defmodule definition scoping the refactor only to the current module instead of the whole file.