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

Whitespace in HTML macro #83

Closed
dbrgn opened this issue Feb 19, 2019 · 7 comments
Closed

Whitespace in HTML macro #83

dbrgn opened this issue Feb 19, 2019 · 7 comments
Assignees

Comments

@dbrgn
Copy link
Collaborator

dbrgn commented Feb 19, 2019

How do I create I the equivalent of Hello <img src="#" alt="Big"> World with the html macro?

When I do html! { <div> Hello <img src="#" alt="Big"> World </div> } then the whitespace around the text is stripped. Can it somehow be escaped without using the text! macro and variable replacement?

@chinedufn
Copy link
Owner

Here's how we're able to parse text without needing quotation marks

return parse_text_node(&mut input);

Basically if we come across a token and it doesn't look like anything else (a tag.. attributes.. etc) then we assume it is text.


When parsing tokens from the compiler we have no idea how much spacing was around the token. We just get raw tokens.

So when we're building a string of text we just insert space between tokens ourselves:

if idx != 0 {
text += " ";
}


As you see in that code we are only inserting a space after a token if it isn't the first text token in the batch of text.

In your example above Hello and World are both the first (and only) text tokens in the batch of text tokens that we see so neither of them is getting any space in front of them.


One thing that would really help here is being able to know the start and end columns of each token

https://docs.rs/proc-macro2/0.4.24/proc_macro2/struct.Span.html#method.start

This way we could look at the two neighboring tokens and know whether or not we need to insert a space character.

Unfortunately this feature of spans hasn't stabilized and I'm not sure if/when it will.


Potential options in the meantime...

  1. Always insert space before and after text tokens. This would mean that if you really didn't want a space between an image and some text after it there would be .a space anyway... but maybe that's a trade-off that we can mark for now.

  2. Add support for literals html! { <div>"This will properly be parsed "</div> }

  1. Remove support for un-quoted text until span starts / ends stabilize
  • fewer surprises but also much less ergonomic IMO. I don't like this and would rather have this bit of sometimes unexpected (on the first and last word in a sequence of text) behavior than throw away the ergonomics here.

Your thoughts?

@dbrgn
Copy link
Collaborator Author

dbrgn commented Feb 19, 2019

Supporting literals sounds like a good idea!

I'm not sure about un-quoted text to be honest. It's strange to have two ways to declare text nodes, but on the other hand I think it's fine as long as the differences are documented (i.e. "white-space is collapsed").

@chinedufn
Copy link
Owner

Hmm I'm thinking that ideally we'd support all ways that someone might try to create things.

So the problem here is less about having two ways and more about having a way that might behave in a way that you don't expect sometimes.

(As always, just my thoughts, very open to being wrong!!)

I can try to figure out where we can follow the progress of stabilization and in the meantime add literal support.

Let me know if you have something different in mind!

@chinedufn
Copy link
Owner

Woah.. wait a second.. this might be possible right now. dtolnay/proc-macro2#166

Doing some reading ...

@chinedufn
Copy link
Owner

chinedufn commented Mar 5, 2019

@dbrgn started working on this - should have a PR ready this week if you can take a look

@chinedufn
Copy link
Owner

Alright all of the test cases are passing and can be seen here

https://github.com/chinedufn/percy/blob/text-space/crates/html-macro-test/src/text.rs

Will need to do some refactoring / clean up of the html-macro and then we'll be good to go!

@dbrgn
Copy link
Collaborator Author

dbrgn commented Mar 6, 2019

@chinedufn fantastic, exactly what I'd intuitively expect! :)

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