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

Syntax error within macro using $(expr) #26

Closed
Nemo157 opened this issue Jan 21, 2016 · 5 comments
Closed

Syntax error within macro using $(expr) #26

Nemo157 opened this issue Jan 21, 2016 · 5 comments

Comments

@Nemo157
Copy link
Contributor

Nemo157 commented Jan 21, 2016

Just encountered a bug very similar to #23, just using $(expr) inside the html instead.

#[test]
fn foo() {
    macro_rules! to_string {
        ($($x:tt)*) => {{
            let mut s = String::new();
            html!(s, $($x)*).unwrap();
            s
        }}
    }

    let name = "Lyra";
    let s = to_string!(p { "Hi, " $(name) "!" });
    assert_eq!(s, "<p>Hi, Lyra!</p>");
}

gives

tests/tests.rs:324:47: 324:48 error: expected `*` or `+`
tests/tests.rs:324     let s = to_string!(p { "Hi, " $(name) "!" });
@Nemo157
Copy link
Contributor Author

Nemo157 commented Jan 21, 2016

As a test I tried changing it to $(name)* and got a syntax error from mauds parser. With a bit of work I managed to get it working with that syntax with both identifiers or expressions inside the () as a workaround 3d8763b.

@Nemo157
Copy link
Contributor Author

Nemo157 commented Jan 22, 2016

The workaround doesn't seem to always work, not sure exactly what triggers the error but when I tried using it in some more complicated macros I would sometimes get error: attempted to repeat an expression containing no syntax variables matched as repeating at this depth. I'm guessing it's ok when it's at the terminal of the macro expansion, but if you have any repetition sequences at the same level it errors out.

I guess the best fix would be very similar to the fix for #21. Although with this it has the additional problem that it's in the usage of the macro, rather than the definition that you'd have to stop using $, i.e.

let s = to_string!(p { "Hi, " #splice (name) "!" });

in the above example. Maybe it would be worth dropping $ entirely and finding an alternative shorthand interpolation syntax to better interact with macro_rules?

@Nemo157
Copy link
Contributor Author

Nemo157 commented Jan 22, 2016

I tried a basic replacement $# and it seems to work (602fdc). The main issue is that it would make any of the special cases like #call basically be keywords, so if you had something like

let (animal, call) = ("cow", "moo");
let s = to_string!(p {
  "The " #animal " says " #call
}

it would fail to parse (or potentially parse with an incorrect output depending on the exact context). I think this is probably ok as it can be worked around with

let (animal, call) = ("cow", "moo");
let s = to_string!(p {
  "The " #animal " says " #(call)
}

and I assume there won't be many special cases added. Any special case where it uses a keyword (like #if let) would be fine anyway as these need to be wrapped already.

@lambda-fairy
Copy link
Owner

To be honest the only reason why I used # in the first place was because it had nice syntax highlighting. My editor would think that #call is a Rust attribute and draw it in a bright red. I used $ for a similar reason, but given the issues around that I'm not very attached to the idea now.

Without that constraint, I think it's better to use ^ for splices and @ for keywords. The former has a connotation of "lifting", and the latter is reminiscent of annotations in Python. This change will free up # for an ID shorthand as well.

Actually it would be cool to have & splice a value by-reference and ^ splice a value by-move. That would cover the use case from #24 very nicely.

What do you think about these ideas?

@Nemo157
Copy link
Contributor Author

Nemo157 commented Feb 2, 2016

Those both sound good to me, I could open a PR with the changes in a couple of hours if you want (unless you wish to do it yourself)?

Nemo157 added a commit to Nemo157/maud that referenced this issue Feb 2, 2016
@Nemo157 Nemo157 mentioned this issue Feb 2, 2016
Nemo157 added a commit to Nemo157/maud that referenced this issue Mar 8, 2016
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

No branches or pull requests

2 participants