rstml 0.12 and braces in attribute values

[blorbb]

follow up to #2753

[blorbb]

The failing check seems to be on something unrelated. Sorry about the bunch of commits, cargo fmt didn't want to format properly for some reason. I can squash some of the commits together if you'd like

[gbj]

I think I'm missing something fundamental here, about why this means that braces need to be required.

If I remove the part of the code that emits an error if I don't have a block in value position, but keep the rest of the changes, I seem to see the same autocomplete improvements that you described initially/with the screenshot in the initial PR.

Is there a different case I should be testing against that shows why requiring a block is better than allowing any expression?

[benwis]

I'll have to wait for blorbb to answer your question, but I believe the desire was to reduce ambiguity by requiring expressions to have braces. Personally I like the idea of the clear separation of rust code and what is basically html

[blorbb]

Hm that's odd, I can't seem to recreate your first screenshot when I remove the errors.

The suggestions in your first screenshot also suggest that you're running a method on a tuple, but abcdefg is not one. Not sure what's happening there.

The second screenshot does work because abcdefg.g is a valid expression so it is expanded correctly. Typing one character to make r-a show suggestions worked in the previous macro too. However, having braces just gives generally better r-a support on any more complex expressions:

The point by @benwis is also true, it would be good to keep clear separation between HTML and rust.

p.s. is there some way to remove all the methods suggested by AriaAttributes, GlobalAttributes, RenderHtml, etc? It makes autocomplete a mess because there are so many methods to search through. It seems like removing them from the prelude works, but the view! macro isn't fully hygenic and depends on these traits being in scope to use their methods. Perhaps remove them from the prelude and make the macro use a fully qualified path? This is a different problem tho, I can make a separate issue/pr.

[sabify]

@gbj is it possible to put it together for a poll?

These changes are really awkward in terms of readability. There are too much of unnecessary braces! If they are mandated, it will reduce readability for sure!

[gbj]

I want to make sure there is a clear benefit here before making it a required breaking change. I tried to run an experiment this morning to test it.

The test: What autocompletion do we get, with and without erroring?
A) Useful autocomplete (includes the .get() method on a ReadSignal)
B) Partial autocomplete (suggests the variable name, includes irrelevant methods but not .get())
C) No autocomplete (does not suggest variable name)

PR as is, with error if no braces
Inside component body: useful autocomplete
Inside view with braces on value.: partial autocomplete (suggests Render and RenderHtml methods, but not .get())
Inside view without braces: no autocomplete (does not suggest variable name)

Video

PR removing the no-braces error, then restarting rust-analyzer
Inside component body: useful autocomplete
Inside view with braces on value.: partial autocomplete (same as above)
Inside view without braces: partial autocomplete (suggests variable name and Render methods etc., but not .get())

Video

It is entirely possible I'm doing something wrong or this is not the intended scenario, but I am not seeing any improvement if it errors when braces are not used. Are you seeing different behavior? It is possible my environment is messed up somehow, I change between so many different branches so frequently that I wouldn't be surprised.

[benwis]

@gbj is it possible to put it together for a poll?

These changes are really awkward in terms of readability. There are too much of unnecessary braces! If they are mandated, it will reduce readability for sure!

If we actually can't find any improvement, perhaps we leave it as a leptosfmt option. However, no other thing in html has multi-word undelineated arguments, so I'm fairly heavy in favor. As mentioned in the PR, this is how Svelte and a couple of others have gone

[blorbb]

Sorry for the late response, haven't had much time recently. I have no clue why .get() is not showing up as an option. I can try figure this out later, though it can probably be a separate issue/PR.

Requiring braces is not necessary for this to work, it's just if you want to enforce a style. Adding braces will just improve r-a support in some cases, but this will work even if braces are not required.
(I initially thought we would have to enforce braces when we were discussing this on discord because parsing chooses to parse braces then fallback to expressions, but in the rstml issue someone raised the very good idea of parsing expressions then falling back to braces instead, which makes a lot more sense)

If you would prefer to not have the breaking change, feel free to remove the lines around the emit_error! and it will probably work just fine!

[gbj]

I initially thought we would have to enforce braces when we were discussing this on discord because parsing chooses to parse braces then fallback to expressions, but in the rstml issue someone raised the very good idea of parsing expressions then falling back to braces instead, which makes a lot more sense

If you would prefer to not have the breaking change, feel free to remove the lines around the emit_error! and it will probably work just fine!

This is great to know, and this makes sense to me.

I think I will plan to

1. remove the error emission, and therefore
2. revert the changes to examples; then
3. document that using braces likely improves r-a support, and is a sensible default, but not required

(It may take me a few days to make these changes and merge the PR; if you'd like to do them instead, that's also very welcome! I will likely get to it this Friday.)

I think opinions on style differ enough (as can be seen even in this thread!) that mandating style changes has the potential to be dangerous to the community/hard for adoption. I am really glad that there is a middle ground.

And thanks again for all your work.

[gbj]

Done separately in #2884 for my sanity, thanks again for your work!