Skip to content

$0 in extract function assist #7793

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

Closed
weigangd opened this issue Feb 26, 2021 · 7 comments
Closed

$0 in extract function assist #7793

weigangd opened this issue Feb 26, 2021 · 7 comments
Labels
A-assists E-easy E-has-instructions Issue has some instructions and pointers to code to get started S-actionable Someone could pick this issue up and work on it right now

Comments

@weigangd
Copy link

weigangd commented Feb 26, 2021

Hello, I am using nvim + LanguageClient-neovim and I have no plugin for snippet support. When I use the "extract function assist" code action, the generated function always has the name $0fun_name. The same happens with nvim-lsp. If I understand #2518 and #4117 correctly, snippet-completion should be disabled automatically if the client does not support snippets.

@Veykril
Copy link
Member

Veykril commented Feb 26, 2021

@Veykril Veykril added A-assists S-actionable Someone could pick this issue up and work on it right now labels Feb 26, 2021
@Veykril
Copy link
Member

Veykril commented Feb 26, 2021

The fix is to check for the capability like done in add_missing_impl_members assist here https://github.com/rust-analyzer/rust-analyzer/blob/d5b8a401f38f88ecb0d894c88dcbab3d338a0ef2/crates/ide_assists/src/handlers/add_missing_impl_members.rs#L121-L140 and add the snippet $0 conditionally.

@Veykril Veykril added E-easy E-has-instructions Issue has some instructions and pointers to code to get started labels Feb 26, 2021
@tomharmon
Copy link

@Veykril you mean like this? That simple? Simply return "" if there isn't that capability, otherwise, do what it already does?

fn format_function(
    ctx: &AssistContext,
    module: hir::Module,
    fun: &Function,
    old_indent: IndentLevel,
    new_indent: IndentLevel,
) -> String {
    let mut fn_def = String::new();

    match ctx.config.snippet_cap {
        None => (),
        Some(_) => {
            let params = make_param_list(ctx, module, fun);
            let ret_ty = make_ret_ty(ctx, module, fun);
            let body = make_body(ctx, old_indent, new_indent, fun);
            format_to!(fn_def, "\n\n{}fn $0{}{}", new_indent, fun.name, params);
            if let Some(ret_ty) = ret_ty {
                format_to!(fn_def, " {}", ret_ty);
            }
            format_to!(fn_def, " {}", body);
        }
    };

    fn_def
}

@Veykril
Copy link
Member

Veykril commented Mar 4, 2021

No not quite, we still want to emit the code but dont want to emit the $0 snippet parts. The example I linked was for how to check whether we want to emit $0 or not.

@Arthamys
Copy link
Contributor

Arthamys commented Mar 5, 2021

I've go the changes in my fork, would that be sufficient to close the issue ?

Arthamys@c118f88

edit

Looking in the extract_function() body, I am wondering if https://github.com/rust-analyzer/rust-analyzer/blob/9707acb59b3f1f74bcc2f806ea5eab59279fad5e/crates/ide_assists/src/handlers/extract_function.rs#L115
should also be changed to

match ctx.config.snippet_cap {
                Some(_) => {
                    builder.insert_snippet(ctx.config.snippet_cap.unwrap(), insert_offset, fn_def)
                }
                None => builder.insert(insert_offset, fn_def),
            }

also, or is this not a "snippet insertion" per se ?

@Veykril
Copy link
Member

Veykril commented Mar 5, 2021

Ye I think that also has to be changed

bors bot added a commit that referenced this issue Mar 5, 2021
7880: Honor snippet capability when using the extract function assist r=lnicola a=Arthamys

This fixes issue #7793

Co-authored-by: san <[email protected]>
@bjorn3
Copy link
Member

bjorn3 commented Mar 5, 2021

Fixed by #7880.

@bjorn3 bjorn3 closed this as completed Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-assists E-easy E-has-instructions Issue has some instructions and pointers to code to get started S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

No branches or pull requests

5 participants