Two CSS formatting bugs in se clean

[gvtulder]

This addresses two small issues with the CSS formatting in se clean.

1. An incorrect space is inserted between two ::, if these colons follow an opening (.

   p:nth-of-type(1)::before{

   is currently changed to

   p:nth-of-type(1): :before{

   The regex responsible for this sees the opening ( and then adds a space after the first : it finds. The comment in the code explains that this is to fix the colon in media queries such as (prefers-color-scheme: dark), but it also affects colons outside the parentheses.

   This can be fixed by making the regex stop at the closing ). I think this shouldn't break anything else. The media query example still works.

2. Strings in the CSS output are not escaped. The raw string from the parser is directly written to the cleaned output.

   If the string contains any quotes or newlines, se clean produces invalid CSS:

   content: "quote \" newline \A ";

   is currently changed to

   content: "quote " newline 
   ";

   The patch uses the tinycss token.representation, which is a quoted and escaped version of the string.

[acabal]

Thanks! I'm not sure if the tests are correct. The in files should be "unclean" CSS, to demonstrate that the golden files then become correctly pretty-printed. So in the in files, you should have various different types of unformatted CSS so we can test how the formatter functions.

[gvtulder]

It depends on what the tests are supposed to do, I think. :)

My intention was to test the changes in this patch. The test is limited to this bugfix, and I think it does that:

• The current implementation fails the test. The string is not escaped correctly, and the ::before selector receives a space between the colons.
• The new implementation passes the test. The string is escaped correctly, and the escapes are correctly normalized (the escaped characters in the output are all capitalized, the diacritics converted to UTF-8, the single quotes normalized to double quotes). There is no space between the colons.
• The space-after-colon-regex still works: if I remove the regex, the output becomes prefers-color-scheme:dark (the space is removed earlier in the function). It's a bit weird, but the fact that the input is the same as the output shows that this part still works.

The test is a bit short, but I think it covers most of the angles: it demonstrates the bug, it demonstrates the fix, and it shows that there is no regression.

Now, you're right, this doesn't test whether "the formatter" still works. But shouldn't that ideally be covered by other tests? I could add some random CSS here, but that would have very little to do with the changes introduced by these two patches.

Are there any specific things you would want to test here?

I believe there are currently no other CSS tests for se clean. So if it helps, I could take some CSS from one of the books, make some changes that need to be prettified, and add that as a new test case. But I would argue that that should be a separate commit, because it's not related to these two bugs.

[acabal]

The tests are for testing the behavior of the tool as a whole, not for testing individual PRs. So, you should update the test so that the tool to performs the expected operations given raw/incorrect input.

[acabal]

You're right that there might not be a test for se clean, so this can be the first one, and for now it just has to demonstrate what this particular patch does. In the future we can add more tests to se clean to fill out expected behavior.

[gvtulder]

Alright, something like this? I also merged the two test-cases now in a single file, so that leaves room for other tests in the future.

[acabal]

Yes, perfect. Thanks!