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

Warn when the style attribute is missing a semicolon ; #182

Open
carson-katri opened this issue May 22, 2024 · 8 comments
Open

Warn when the style attribute is missing a semicolon ; #182

carson-katri opened this issue May 22, 2024 · 8 comments
Assignees

Comments

@carson-katri
Copy link
Contributor

carson-katri commented May 22, 2024

A common mistake is writing a style attribute with spaces as delimiters instead of semicolons ;:

<Element style="padding() background(.red)" />

This should log a warning, ideally pointing to the position where the semicolon should be inserted. A message similar to this could be used:

Missing semicolon between modifiers in `style` attribute:

<Element style="padding() background(.red)" />
                         ^ add a `;` semicolon after `padding()`

home_live.swiftui.neex:1:25

Alternatively, spaces could be allowed as a delimiter as well or instead of semicolons.

@bcardarella bcardarella self-assigned this May 22, 2024
@bcardarella
Copy link
Contributor

I just compared to web and in Chrome I don't see any support for whitespace deliminator between style rules so I'd like to not support that in ours.

But I agree some warning would be nice. @NduatiK I think we'd need to support that in the parser itself as currently I'm just doing String.split(style, ";")

@NduatiK
Copy link

NduatiK commented Aug 1, 2024

Let's clarify:

  • you want a warning when the parser finds rules that are only delimited by whitespaces
  • the warning should only happen inside style attributes
  • warnings should be configurable from the caller, enabled on styles, disabled everywhere else (sheets)

@bcardarella, did I get that right?

@bcardarella
Copy link
Contributor

so currently I split the value of the style attr on ; and just iterate through each rule one by one and send it to the parser. What I think we should do instead is send the entire string to the parser. The current RULES parser is splitting on newline, but we should probably introduce the optional ; to deliminate between rules:

padding(); background(.red)

should be the same as:

padding(); background(.red);

and

padding()
background(.red)

and

padding();
background(.red);

Does that make sense?

@NduatiK
Copy link

NduatiK commented Aug 1, 2024

Makes sense.
One more clarification, would a missing semicolon be expected on sheets and produce a warning?

@bcardarella
Copy link
Contributor

are you asking if the following:

padding();
fooBar()
barBaz();

where the 2nd rule has no semicolor?

@NduatiK
Copy link

NduatiK commented Aug 1, 2024

Yes, should the SHEET sigil produce a warning if the 2nd rule's semicolon is missing?

I'd like to know if warnings should always be produced or if it should be configurable.

@bcardarella
Copy link
Contributor

That's a good question, are semicolons required for multiline in css or not? I forget what the current state of that is. If it is then we should probably just follow that but it may need to be a v0.4 thing and accept the breaking change

@cmnstmntmn
Copy link
Contributor

Going to link this issue liveview-native/liveview-client-swiftui#1421, since they might use the same parser internally.

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

4 participants