Skip to content

Conversation

@xNul
Copy link
Contributor

@xNul xNul commented Sep 4, 2021

rust_analyzer_pr
Adds support for valid Rust completion of raw identifiers.

Previously, code completion of fields made via raw identifiers would not re-insert those raw identifiers, resulting in invalid Rust code. Now, code completion of fields made via raw identifiers do re-insert those raw identifiers, resulting in valid Rust code.

The same is true for all code completion instances for fields and compatible Rust identifiers.

@lnicola
Copy link
Member

lnicola commented Sep 6, 2021

It might be worth renaming name to mod_name and adding a new function that does this.

Copy link
Contributor

@matklad matklad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors r+

LGTM as is!

@matklad
Copy link
Contributor

matklad commented Sep 6, 2021

It might be worth renaming name to mod_name and adding a new function that does this.

@lnicola could you expand on this? I don't quite understand what do you mean here :)

@lnicola
Copy link
Member

lnicola commented Sep 6, 2021

This method here doesn't have a great name and there's a bit of duplication between this PR and raw_ident_esc.

@bors
Copy link
Contributor

bors bot commented Sep 6, 2021

@bors bors bot merged commit cbc13ae into rust-lang:master Sep 6, 2021
@matklad
Copy link
Contributor

matklad commented Sep 6, 2021

Ah... The method has the right name I think -- it is supposed to be used as make::name, and it is named after the return type (ast::Name), like any other method out there. Ideally, we'd auto-generate all those!

And yeah, ideally we should move raw_ident_esc to some other place, not sure what that would be!

@xNul
Copy link
Contributor Author

xNul commented Sep 6, 2021

@lnicola I considered not duplicating and instead, creating a public helper function such as

pub fn raw_ident_verify(ident: &str) -> bool {
    let is_keyword = parser::SyntaxKind::from_keyword(ident).is_some();
    is_keyword && !matches!(ident, "self" | "crate" | "super" | "Self") 
}

which is used by both render_field and raw_ident_esc but decided not to when I read the "Scale of Changes" section in the dev docs. I have no problem with changing it though.

@lnicola
Copy link
Member

lnicola commented Sep 6, 2021

The function name (heh) is fine, but it returns a module declaration.

@matklad
Copy link
Contributor

matklad commented Sep 6, 2021

pub fn name(text: &str) -> ast::Name {

it returns ast::Name?

@lnicola
Copy link
Member

lnicola commented Sep 6, 2021

Ooh, right 🤦. It parses a module declaration but returns only the name. Don't mind me.

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.

3 participants