Skip to content
This repository has been archived by the owner on Jun 30, 2024. It is now read-only.

support nextjs <style> tag attributes in highlighting #340

Closed
Pika-Pool opened this issue Dec 10, 2021 · 23 comments · Fixed by #346
Closed

support nextjs <style> tag attributes in highlighting #340

Pika-Pool opened this issue Dec 10, 2021 · 23 comments · Fixed by #346
Assignees

Comments

@Pika-Pool
Copy link
Contributor

this extension works great for nextjs <style jsx> tag, but breaks for global styles:
index.js file

Could you add support for this?

@jasonwilliams
Copy link
Collaborator

@Pika-Pool would you be interested in working on this feature? See what I did here 5e70d6f

@Pika-Pool
Copy link
Contributor Author

Yes, I'll try to implement this.

@jasonwilliams
Copy link
Collaborator

It should be just updating the Regex here then also updating the test too or adding a new test.

We have a guide to follow for adding syntax highlighting changes https://github.com/styled-components/vscode-styled-components/blob/master/CONTRIBUTING.md#adding-syntax

@Pika-Pool
Copy link
Contributor Author

Pika-Pool commented Dec 13, 2021

I made a regex to match when there is a jsx attribute on a style tag. The regex seems to work on regex101.com: https://regex101.com/r/JXStXF/1/. But vscode does not match it. Any clues as to why this is happening?

The pattern I'm using:

    {
      "contentName": "source.css.scss",
      "begin": "<style\\s+(?=(?:(?:[^\\>\\s]+\\s+)+jsx|jsx)(?:\\s+|\\>)).*\\>{(`)",
      "beginCaptures": {
        "0": {
          "name": "punctuation.definition.string.template.begin.js string.template.js"
        }
      },
      "end": "`(?=\\}\\<\\/style\\>)",
      "endCaptures": {
        "0": {
          "name": "punctuation.definition.string.template.end.js string.template.js"
        }
      },
      "patterns": [
        {
          "include": "source.css.styled"
        }
      ]
    }

VSCode Screenshot

This regex even works on vscode's find feature, as shown in the following screenshot
regex working in vscode's find feature

@jasonwilliams
Copy link
Collaborator

jasonwilliams commented Dec 13, 2021

Thanks for jumping on this and getting involved, it’s appreciated.

Do you need the final backtick to be in a capture group? The original Regex didn’t do that. Edit: ok I see you’ve set it to be the first capturing group so that seems ok. I will take a proper look when I get time.
it could be that 0 matches the whole thing and not the first capturing group.

Also, stupid Q but are you set up correctly? It’s using your local version of the extension and not the installed one?

@Pika-Pool
Copy link
Contributor Author

I'm pretty sure its using the local version, because when I rerun after putting the original regex, it shows the changes.

This is what I did:

  • Enable the builtin extension ms-vscode.js-debug
  • press F5 to start debugging, which ran the npm: watch task, and then opened a new vscode window, with the colorize-fixtures directory opened in it

@jasonwilliams
Copy link
Collaborator

ok sounds right

Try changing the begin captures to 1 instead of 0 see if that changes anything

@Pika-Pool
Copy link
Contributor Author

That doesn't change anything

@Pika-Pool
Copy link
Contributor Author

Tried putting everything except `(backtick) in a non-capturing group: (?:<style\\s+(?=(?:(?:[^>\\s]+\\s+)+jsx|jsx)(?:\\s+|>))[^>]*>\\{)()`. Still doesn't work in vscode. Works in regex101 https://regex101.com/r/pHtIvk/1/

@jasonwilliams
Copy link
Collaborator

jasonwilliams commented Dec 14, 2021

This works for me.
https://regex101.com/r/p7JVlT/1/

I know it triggers a pattern error but it works in VSCode. Precision isn't amazing but it should do.

Escaped:
"begin": "(?<=<style\\s(jsx|global|\\s)*\\>\\{)`",

Screenshots:
image

image

image

I'm not sure why yours doesn't work, but it doesn't just break the regex for that rule it breaks all of them. So maybe it needs to be simplified down or something. Internally its breaking and silently erroring

@jasonwilliams jasonwilliams changed the title support nextjs <style> tag highlighting support nextjs <style> tag attributes in highlighting Dec 14, 2021
@Pika-Pool
Copy link
Contributor Author

This is a much cleaner solution. Though, it matches the style tag even if the jsx attribute is not set. Do we want that?

Is there a way to test/see why my solution is failing? Cause it does work in the search tab of vscode, so why not in the extension?

Also, is src/tests/suite/colorize-results/nextStyle_js.json automatically generated or hard coded?

@jasonwilliams
Copy link
Collaborator

jasonwilliams commented Dec 15, 2021

This is a much cleaner solution. Though, it matches the style tag even if the jsx attribute is not set. Do we want that?

that’s easily fixed by changing the asterisk to a plus. Good spot

Is there a way to test/see why my solution is failing? Cause it does work in the search tab of vscode, so why not in the extension?

I believe the engine used in find and the one used for syntax highlighting are 2 different engines. Find is ripgrep, syntax highlighting is https://github.com/kkos/oniguruma

Also, is src/tests/suite/colorize-results/nextStyle_js.json automatically generated or hard coded?

automatically generated when you run npm test

@jasonwilliams
Copy link
Collaborator

@Pika-Pool was you still looking into this? Do my answers help?

@Pika-Pool
Copy link
Contributor Author

Yes, I'm still looking into it. Just stuck with some work.

that’s easily fixed by changing the asterisk to a plus. Good spot

It still has the same issue. It'll be a match if only a global attribute is added. I came up with this final solution: https://regex101.com/r/17PV9d/1/ . It works on vscode too. screenshot:
Screenshot

I believe the engine used in find and the one used for syntax highlighting are 2 different engines. Find is ripgrep, syntax highlighting is https://github.com/kkos/oniguruma

I actually tried my previous solution with https://github.com/atom/node-oniguruma, a node module for oniguruma. It actually gives 2 captures, even if I use a non-capturing group (?:). I don't understand how to omit/define the first group in the beginCaptures.

Anyway, for now, I guess the above solution works. I'll put a PR for that.

@Pika-Pool
Copy link
Contributor Author

Getting this error on running npm test for the first time. Running it again, all tests pass. This is the way its supposed to be right?

...
Downloading VS Code 1.63.1 from https://update.code.visualstudio.com/1.63.1/win32-archive/stable: 106086389/106129027 (10Downloading VS Code 1.63.1 from https://update.code.visualstudio.com/1.63.1/win32-archive/stable: complete
Downloaded VS Code 1.63.1 into D:\vscode-styled-components\.vscode-test\vscode-win32-archive-1.63.1


[main 2021-12-16T09:08:03.730Z] update#setState idle

[main 2021-12-16T09:08:04.789Z] ExtensionHostStarterWorker created

[main 2021-12-16T09:08:09.531Z] Starting extension host with pid 17744 (fork() took 155 ms).

(node:17744) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
(Use `Code --trace-deprecation ...` to show where the warning was created)



  colorization

    ✔ arrow-function.js (1253ms)

    ✔ attrs.js (609ms)

    ✔ comment.js (262ms)

[main 2021-12-16T09:08:22.312Z] CodeWindow: failed to load (reason: <unknown>, code: -3)

[main 2021-12-16T09:08:22.388Z] CodeWindow: failed to load (reason: <unknown>, code: -3)

    ✔ component.js (344ms)

    ✔ createGlobalStyle.js (271ms)

    ✔ css-prop.js (362ms)

    ✔ custom-at-rule.js (300ms)

    ✔ dot-tag.js (273ms)

    ✔ export-default.js (319ms)

    ✔ extend.js (311ms)

    ✔ function-call-space.js (310ms)

    ✔ function-call-with-css-helper.js (385ms)

[main 2021-12-16T09:08:25.115Z] CodeWindow: failed to load (reason: <unknown>, code: -3)

    ✔ function-call.js (250ms)

    ✔ function-media-queries.js (793ms)

[main 2021-12-16T09:08:26.204Z] CodeWindow: failed to load (reason: <unknown>, code: -3)

    ✔ injectGlobal.js (317ms)

    ✔ inside-function.js (253ms)

[main 2021-12-16T09:08:26.794Z] CodeWindow: failed to load (reason: <unknown>, code: -3)

    ✔ inside-method.js (363ms)

    ✔ keyframes.js (304ms)

[main 2021-12-16T09:08:27.399Z] CodeWindow: failed to load (reason: <unknown>, code: -3)

    ✔ mixin.js (229ms)

    ✔ nested-template-strings-with-helper.js (625ms)

    1) nextStyle.js

    ✔ object-literal.js (254ms)

[main 2021-12-16T09:08:28.946Z] CodeWindow: failed to load (reason: <unknown>, code: -3)

    ✔ segmented-component.js (236ms)

[main 2021-12-16T09:08:29.365Z] CodeWindow: failed to load (reason: <unknown>, code: -3)

    ✔ string-tagname.js (204ms)

[main 2021-12-16T09:08:29.526Z] CodeWindow: failed to load (reason: <unknown>, code: -3)

    ✔ stylesheet.js (215ms)

[main 2021-12-16T09:08:29.666Z] CodeWindow: failed to load (reason: <unknown>, code: -3)

[main 2021-12-16T09:08:29.828Z] CodeWindow: failed to load (reason: <unknown>, code: -3)

    ✔ theme-function.js (268ms)

    ✔ typescript-attr.ts (543ms)

    ✔ typescript-css.ts (417ms)

    ✔ typescript-emotion.ts (349ms)

[main 2021-12-16T09:08:31.285Z] CodeWindow: failed to load (reason: <unknown>, code: -3)

[main 2021-12-16T09:08:31.319Z] CodeWindow: failed to load (reason: <unknown>, code: -3)

    ✔ typescript-multiline.ts (266ms)

[main 2021-12-16T09:08:31.624Z] CodeWindow: failed to load (reason: <unknown>, code: -3)

    ✔ typescript-tag.ts (215ms)

[main 2021-12-16T09:08:31.841Z] CodeWindow: failed to load (reason: <unknown>, code: -3)

    ✔ typescript.ts (217ms)

    ✔ variable-assignment.js (221ms)

[main 2021-12-16T09:08:32.204Z] CodeWindow: failed to load (reason: <unknown>, code: -3)

    ✔ white-space-property.js (199ms)

    ✔ withComponent.js (363ms)

[main 2021-12-16T09:08:32.821Z] CodeWindow: failed to load (reason: <unknown>, code: -3)

    ✔ withConfig.js (252ms)

  35 passing (13s)

  1 failing

  1) colorization
       nextStyle.js:

      AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
+ actual - expected ... Lines skipped

  [
    {
...
    },
    {
+     c: '  ',
-     c: '    ',
      r: {
        dark_plus: 'default: #D4D4D4',
...
    },
    {
+     c: '    ',
-     c: '      ',
      r: {
        dark_plus: 'default: #D4D4D4',
...
    },
    {
+     c: '      ',
-     c: '        ',
      r: {
        dark_plus: 'default: #D4D4D4',
...
    },
    {
+     c: '{',
-     c: '<',
      r: {
+       dark_plus: 'punctuation.section.embedded: #569CD6',
+       dark_vs: 'punctuation.section.embedded: #569CD6',
+       hc_black: 'punctuation.section.embedded: #569CD6',
+       light_plus: 'punctuation.section.embedded: #0000FF',
+       light_vs: 'punctuation.section.embedded: #0000FF'
-       dark_plus: 'punctuation.definition.tag: #808080',
-       dark_vs: 'punctuation.definition.tag: #808080',
-       hc_black: 'punctuation.definition.tag: #808080',
-       light_plus: 'punctuation.definition.tag: #800000',
-       light_vs: 'punctuation.definition.tag: #800000'
      },
+     t: 'source.js meta.function.js meta.block.js meta.tag.js meta.jsx.children.js punctuation.section.embedded.begin.js'
-     t: 'source.js meta.function.js meta.block.js meta.tag.js meta.jsx.children.js meta.tag.js punctuation.definition.tag.begin.js'
    },
    {
+     c: '/*',
-     c: 'style',
      r: {
+       dark_plus: 'comment: #6A9955',
+       dark_vs: 'comment: #6A9955',
+       hc_black: 'comment: #7CA668',
+       light_plus: 'comment: #008000',
...
-       dark_plus: 'entity.name.tag: #569CD6',
-       dark_vs: 'entity.name.tag: #569CD6',
-       hc_black: 'entity.name.tag: #569CD6',
-       light_plus: 'entity.name.tag: #800000',
...
      + expected - actual


        at D:\vscode-styled-components\src\tests\suite\colorization.test.js:33:20
        at processTicksAndRejections (internal/process/task_queues.js:93:5)


Error: 1 tests failed.

[main 2021-12-16T09:08:33.151Z] Waiting for extension host with pid 17744 to exit.

[main 2021-12-16T09:08:33.296Z] Extension host with pid 17744 exited with code: 1, signal: null.

Exit code:   1
Done

Failed to run tests

@jasonwilliams
Copy link
Collaborator

jasonwilliams commented Dec 16, 2021

Yep that’s normal, the second run will use the new generated file.

make sure you put a link to the regex101 in your PR, it will be useful for the commit message

@Pika-Pool
Copy link
Contributor Author

husky won't let me commit as many other files are not formatted as per prettier. Should I add a lint commit too?

@jasonwilliams
Copy link
Collaborator

Yeah prettier was updated recently, I’m surprised I didn’t have that problem though? How many files are we talking?

@Pika-Pool
Copy link
Contributor Author

Pika-Pool commented Dec 16, 2021

npm run lint shows 15 files

> prettier --check .
[warn] .vscode\launch.json
[warn] .vscode\settings.json
[warn] .vscode\tasks.json
[warn] build.js
[warn] CHANGELOG.md
[warn] CONTRIBUTING.md
[warn] css.styled.configuration.json
[warn] package.json
[warn] README.md
[warn] src\colorProvider.ts
[warn] src\completionItemProvider.ts
[warn] src\extension.ts
[warn] src\insertColonCommand.ts
[warn] syntaxes\css.styled.json
[warn] tsconfig.json
[warn] Code style issues found in the above file(s). Forgot to run Prettier?
husky - pre-commit hook exited with code 1 (error)

Pika-Pool added a commit to Pika-Pool/vscode-styled-components that referenced this issue Dec 16, 2021
…ed-jsx

- fixes styled-components#340
- fix issue where adding the `global` attribute break highlighting
@Pika-Pool
Copy link
Contributor Author

Nevermind, it was end-of-line issue. Created a PR

@jasonwilliams
Copy link
Collaborator

ah we have editorConfig for that, are you using the editorConfig plugin?
https://github.com/styled-components/vscode-styled-components/blob/master/.editorConfig#L4

jasonwilliams pushed a commit that referenced this issue Dec 16, 2021
@Pika-Pool
Copy link
Contributor Author

This was my first merged PR. Thanks you so much for ur help @jasonwilliams. Really enjoyed contributing. Will try and contribute more in future

@jasonwilliams
Copy link
Collaborator

jasonwilliams commented Dec 17, 2021

Thanks @Pika-Pool , we have a lot of bugs like this and i don't really have the time to solve them all.

If you could solve this one for example #292 you'd be a hero haha.

I like the idea of using https://github.com/atom/node-oniguruma I will certainly try that myself in future, and i have updated the contributing guide.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants