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

Support r#raw_identifiers #932

Closed
kovaxis opened this issue Oct 26, 2024 · 7 comments
Closed

Support r#raw_identifiers #932

kovaxis opened this issue Oct 26, 2024 · 7 comments
Labels
bug c: register Register classes, functions and other symbols to GDScript status: wontfix This will not be worked on

Comments

@kovaxis
Copy link

kovaxis commented Oct 26, 2024

Hello!

I wanted to have a function named box, so my first natural instinct was to do:

#[godot_api]
impl MyType {
    #[func]
    fn r#box() {}
}

Using raw identifiers.

When I went to use the function from GDScript, I expected to see a function named box, but instead I got a function named r#box.

Note: I then learned that you can do this instead:

#[godot_api]
impl MyType {
    #[func(rename=box)]
    fn box_func() {}
}

Which is not bad at all, but it is not the "native" way to use reserved names in Rust.

@Bromeon Bromeon added bug c: register Register classes, functions and other symbols to GDScript labels Oct 26, 2024
@Bromeon
Copy link
Member

Bromeon commented Oct 26, 2024

Thanks! Looks like venial returns the r# prefix; not sure if we should address it there or on our side.

The one thing I'm worried about: we technically need to do this for all possible identifiers, whether functions, fields, constants, types, parameters, structs, enums, maybe even macros or attributes? This would require huge changes for very little benefit (and using keywords for identifiers is typically not best practice, plus it can be worked around with rename). 🤔

@PgBiel
Copy link
Contributor

PgBiel commented Oct 26, 2024

I'd say this isn't our fault. There should be a way in venial to retrieve the identifier regardless of the r# prefix.

@kovaxis
Copy link
Author

kovaxis commented Oct 26, 2024

I hadn't heard of venial before. I believe venial should be stripping the r# prefix before handing out the identifier, since in rust r#identifier is supposed to be identical to identifier.

@kovaxis
Copy link
Author

kovaxis commented Oct 26, 2024

I'm not sure what is the best way to report this in venial. I think it's best if you open up an issue on their repo, since you know more details about their API.

@Bromeon
Copy link
Member

Bromeon commented Nov 3, 2024

There aren't many contributors to venial at the moment, a while ago I helped out with lots of large-scale additions. So your best chances are to open a PR, I could also help review it 😉

@lilizoey
Copy link
Member

lilizoey commented Dec 9, 2024

I hadn't heard of venial before. I believe venial should be stripping the r# prefix before handing out the identifier, since in rust r#identifier is supposed to be identical to identifier.

i dont think you could really do this, since then you'd get an error if you then tried to use the raw identifier in a macro somewhere. like say you had a macro identity that just gives the exact same code out as it takes in. then this code would break:

fn main() {
  let identity!(r#let) = 10;
}

since it'd become let let = 10 (which doesnt compile) as opposed to let r#let = 10 (which compiles).

@Bromeon
Copy link
Member

Bromeon commented Dec 30, 2024

The one thing I'm worried about: we technically need to do this for all possible identifiers, whether functions, fields, constants, types, parameters, structs, enums, maybe even macros or attributes? This would require huge changes for very little benefit (and using keywords for identifiers is typically not best practice, plus it can be worked around with rename).

This still stands. Unless someone is willing to demonstrate that this is possible to implement without significantly worsening maintenance, I don't think the small benefit is worth the downsides.

@Bromeon Bromeon closed this as not planned Won't fix, can't repro, duplicate, stale Dec 30, 2024
@Bromeon Bromeon added the status: wontfix This will not be worked on label Dec 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug c: register Register classes, functions and other symbols to GDScript status: wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

4 participants