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

Completing enum variants fills with parentheses #13767

Closed
ntBre opened this issue Dec 13, 2022 · 4 comments
Closed

Completing enum variants fills with parentheses #13767

ntBre opened this issue Dec 13, 2022 · 4 comments
Labels
A-completion autocompletion C-feature Category: feature request

Comments

@ntBre
Copy link
Contributor

ntBre commented Dec 13, 2022

Currently, enum variants get completed with parentheses inside the variant. This completion also leaves the cursor after the closing parentheses, forcing me to move back into the parentheses, delete everything inside, and then type what I actually want in there.

Example of the issue

I would much rather the completion stop at Enum::Variant, allowing me to type the parentheses and their contents, or at most, for the completion snippet to leave me inside the parentheses. Could some kind of configuration option be added for this? If someone could point me in the right direction, I would be glad to try to work on this myself too.

I initially posted this as an Emacs lsp-mode issue but was pointed over here.

@flodiebold
Copy link
Member

IMO if snippets aren't enabled we shouldn't be adding any parentheses at all, as we do for function calls; if snippets are enabled, we should still have a setting like for function calls to control whether to add snippets, parentheses or nothing. Or maybe we should just respect the callable snippet setting.

For functions this happens here:

pub(super) fn add_call_parens<'b>(

note that this function already only gets called if snippets are enabled.
The code that would need to be changed is here:
pub(crate) fn render_tuple_lit(
db: &dyn HirDatabase,
snippet_cap: Option<SnippetCap>,
fields: &[hir::Field],
path: &str,
) -> RenderedLiteral {
let completions = fields.iter().enumerate().format_with(", ", |(idx, _), f| {
if snippet_cap.is_some() {
f(&format_args!("${{{}:()}}", idx + 1))
} else {
f(&format_args!("()"))
}
});
let types = fields.iter().format_with(", ", |field, f| f(&field.ty(db).display(db)));
RenderedLiteral {
literal: format!("{}({})", path, completions),
detail: format!("{}({})", path, types),
}
}

@flodiebold flodiebold added C-feature Category: feature request A-completion autocompletion labels Dec 13, 2022
@ntBre
Copy link
Contributor Author

ntBre commented Dec 13, 2022

IMO if snippets aren't enabled we shouldn't be adding any parentheses at all, as we do for function calls; if snippets are enabled, we should still have a setting like for function calls to control whether to add snippets, parentheses or nothing. Or maybe we should just respect the callable snippet setting.

I definitely agree. I have lsp-rust-analyzer-completion-add-call-parenthesis set to nil, and I think I have snippets enabled otherwise, so it would be great to have a similar setting for this.

Thank you very much for the code pointers! I'll try to dive in later today.

ntBre added a commit to ntBre/rust-analyzer that referenced this issue Dec 20, 2022
@ntBre
Copy link
Contributor Author

ntBre commented Dec 20, 2022

Sorry for the delay on this, I finally got some time to look into it. I added a commit that handles the very basic version of what I wanted, but it certainly doesn't cover everything we've talked about. I also couldn't figure out how to update the label on the completion, so it still says "Variant(...)" even though it only completes "Variant" as I wanted.

I guess I will need to make the signature of render_tuple_lit more like that of add_call_parens and pass more information around. Does that sound right to you? Thanks again for all of the pointers!

@ntBre
Copy link
Contributor Author

ntBre commented Dec 20, 2022

I looked more closely and added another commit taking care of the label. I'm now happy with the changes for my use case at least. I think now the only part missing is handling the case with snippets enabled.

bors added a commit that referenced this issue Dec 20, 2022
Complete enum variants without parens when snippets are disabled

This handles the portion of #13767 that bothered me, but I can try to work on the other parts we discussed if needed.
@ntBre ntBre closed this as completed Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-completion autocompletion C-feature Category: feature request
Projects
None yet
Development

No branches or pull requests

2 participants