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: code action for refactoring UnlessWithElse #35

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

CoderDennis
Copy link
Contributor

Issue #17

This is a very rough draft. Just experimenting with the AST transforms and haven't chosen good variable names for everything yet. The adjust_comment_lines functions represent something I tried that didn't help.

@CoderDennis
Copy link
Contributor Author

@mhanberg This code assumes that the diagnostic from Credo contains the whole text of the unless and else blocks that would need to be refactored. Based on what I've gathered around the fact that only the first character gets highlighted, I'm guessing that may not be the case. I had started down the path of inspecting the diagnostic struct but ran into issues running the locally built version as we discussed elsewhere.

@mhanberg
Copy link
Contributor

mhanberg commented Jun 5, 2023

So running it locally should work on main now if you rebase.

You can just do bin/start --port 9000 in the credo-language-server repo and then open your editor to any arbitrary repo.

With regard to the diagnostic, yes, Credo only tells you the line and col where the issue begins.

It might be sufficient to get the AST node that begins at that position (the entire document should be stored in the gen_lsp state) and work from there.

I have been considering patching Credo and upstreaming it, but that would obviously only work for the latest version (as well as the Credo codebase is... a beast, so might be hard to patch it in).

@mhanberg
Copy link
Contributor

mhanberg commented Jun 7, 2023

@mhanberg This code assumes that the diagnostic from Credo contains the whole text of the unless and else blocks that would need to be refactored. Based on what I've gathered around the fact that only the first character gets highlighted, I'm guessing that may not be the case. I had started down the path of inspecting the diagnostic struct but ran into issues running the locally built version as we discussed elsewhere.

I'd have to double check, but an issue has a trigger field on it that might have the source code you want to manipulate

edit: actually it looks like in this case, it would just be "unless"

@mhanberg
Copy link
Contributor

mhanberg commented Jun 7, 2023

Related: rrrene/credo#1047

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants