Generate enum variant assist #12334
Generate enum variant assist #12334
Conversation
This also disables "generate function" when what we clearly want is to generate an enum variant. Co-authored-by: Maarten Flippo <maartenflippo@outlook.com>
| } | ||
|
|
||
| let name_ref = path.segment()?.name_ref()?; | ||
| if name_ref.text().as_str().chars().next()?.is_ascii_lowercase() { |
There was a problem hiding this comment.
| if name_ref.text().as_str().chars().next()?.is_ascii_lowercase() { | |
| if name_ref.text().as_str().starts_with(char::is_lowercase) { |
Similar can be done for the uppercase check in generate_function. Going with the non-ascii version probably makes more more sense here (though I don't really know of anyone NOT using the latin alphabet set for identifiers so it really shouldn't matter)
There was a problem hiding this comment.
Done in 796c4d8
I suppose there is no need to say this anymore 😄 Since you have included the original commit author for the rebased commit I personally think this is fine. Thanks! You are more than welcome to pick up your own issues from here on of course (and ask for guidance/mentoring on some that seem interesting to you :) ) @bors delegate+ |
|
✌️ @fasterthanlime can now approve this pull request |
| let text = format!("{}{},\n{}", prefix, name_ref, enum_indent_level); | ||
|
|
||
| acc.add( | ||
| AssistId("generate_enum_variant", AssistKind::Generate), | ||
| "Generate variant", | ||
| target, | ||
| |builder| builder.insert(offset, text), | ||
| ) |
There was a problem hiding this comment.
Ah, missed this. Let's pull the text creation into the closure since that is evaluated lazily.
There was a problem hiding this comment.
Done in ae2c0db
|
Took care of those last two review comments — the only thing I'm still unhappy with it the indentation in the method generation assist, but I suppose that's for a separate PR: rust-analyzer/crates/ide-assists/src/handlers/generate_function.rs Lines 1762 to 1784 in ae2c0db For now I just made the test match the assist's output. |
|
Indentation and whitespace formatting is a big annoyance for us in assists still. We try to fix that up to some degree manually (within reasonable limits of code readability). So having non-perfect output for these is alright for the time being. We are planning on having a proper formatter that works on our syntax trees in the future to get rid of all those manual fixups. Thanks! |
|
📌 Commit ae2c0db has been approved by |
|
☀️ Test successful - checks-actions |
generate-variant.mp4 |
So, this is kind of a weird PR!
I'm a complete newcomer to the
rust-analyzercodebase, and so I browsed the "good first issue" tag, and found #11635. Then I found two separate folks had taken stabs at it, most recently @maartenflippo — and there had been a review 3 days ago, but no activity in a little while, and the PR needed to be rebased since the crates were renamed fromsnake_casetokebab-case.So to get acquainted with the codebase I typed this PR by hand, looking at the diff in #11995, and I also added a doc-test (that passes).
I haven't taken into account the comments @Veykril left in #11995, but I don't want to steal any of @maartenflippo's thunder! Closing this PR is perfectly fine. Or Maarten could use it as a "restart point"? Or I could finish it up, whichever feels best to everyone.
I think what remains to be done in this PR, at least, is:
PascalCasePascalCaseadt.source()as mentioned here: fix: skip the generate function assist when dealing with an enum variant #11995 (comment)Anyway. This PR can go in any direction. You can tell me "no, tackle your own issue!" And I'll go do that and still be happy I got to take a look at rust-analyzer some by doing this. Or you can tell me "okay, now you finish it", and I guess I'll try and finish it :)
Closes #11635