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

(javascript) Incorrect highlight after '/' symbol #2530

Closed
gdmfilippov opened this issue May 5, 2020 · 10 comments
Closed

(javascript) Incorrect highlight after '/' symbol #2530

gdmfilippov opened this issue May 5, 2020 · 10 comments

Comments

@gdmfilippov
Copy link

@gdmfilippov gdmfilippov commented May 5, 2020

Describe the issue
In some cases, the highlight is not correct after the '/' symbol.
Bellow is 2 examples.

Which language seems to have the issue?*
javascript

Are you using highlight or highlightAuto?
highlight

Sample Code to Reproduce
Example 1:

const x = `${duration / 1000}`;
func();

Example 2:

a(b, /.*c.*/);
func();

Expected behavior
In the example 1, the text after the '/' symbol doesn't change highlight color (including the symbol itself)

In the example 2 highlight after regexp must be correct

Additional context
It seems, problem appears in 9.18.1. I can't reproduce the problem with 9.18.2

@joshgoebel
Copy link
Member

@joshgoebel joshgoebel commented May 5, 2020

It seems, problem appears in 9.18.1. I can't reproduce the problem with 9.18.2

Please confirm you are actually having this problem with the latest release, 10.0.2.

@joshgoebel
Copy link
Member

@joshgoebel joshgoebel commented May 5, 2020

If you can test #2531, please do.

@joshgoebel
Copy link
Member

@joshgoebel joshgoebel commented May 5, 2020

Nope that just shifts it, the problem is that value container and attr container are both fighting for this content...

@joshgoebel
Copy link
Member

@joshgoebel joshgoebel commented May 5, 2020

Where else is the ident:as in \b[ident][colon] pattern used in JS other than object keys? If we just removed the attr container and looked for keys anywhere, what would that break?

@joshgoebel
Copy link
Member

@joshgoebel joshgoebel commented May 5, 2020

Answer: ternary, lol.

key4: false ? undefined : true

Right there in the test file too, LOL.

@joshgoebel
Copy link
Member

@joshgoebel joshgoebel commented May 5, 2020

FYI: Example 1 should not ben an issue in version 10, can you confirm?

@joshgoebel
Copy link
Member

@joshgoebel joshgoebel commented May 5, 2020

      { // object attr container
        begin: regex.concat(/[{,\n]\s*/,
          // we need to look ahead to make sure that we actually have an
          // attribute coming up so we don't steal a comma from a potential
          // "value" container
          regex.lookahead(regex.concat(
            // we also need to allow for multiple possible comments inbetween
            // the first key:value pairing
            /(\/\/.*\s*)*/,
            IDENT_RE + '\\s*:'))),

That's one way to get it done...

@joshgoebel
Copy link
Member

@joshgoebel joshgoebel commented May 5, 2020

Probably also needs to check multi-line comments.

@gdmfilippov
Copy link
Author

@gdmfilippov gdmfilippov commented May 6, 2020

FYI: Example 1 should not ben an issue in version 10, can you confirm?

Example 1 has an issue in version 10, but example 2 - not:
Version 9.18.1: https://jsfiddle.net/j35arp2d/11/
Version 10.0.2: https://jsfiddle.net/2us07vdh/1/

If you can test #2531, please do.
Will try to test in 24 hours

@joshgoebel
Copy link
Member

@joshgoebel joshgoebel commented May 6, 2020

Example 1 has an issue in version 10, but example 2 - not:

I see the opposite. In version 10 example 1 looks great while example 2 is broken.

@joshgoebel joshgoebel added this to the 10.1 milestone May 7, 2020
qtprojectorg pushed a commit to qtqa/gerrit that referenced this issue May 11, 2020
This fixes an issue where things were incorrectly highlighted after
the '/' symbol in javascript [1].

This also includes other fixes to the java language, c language
and php language.

[1] highlightjs/highlight.js#2530

Bug: Issue 12726
Change-Id: I7fe08646e2698fc3f4e6025cf7a4201565a84635
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants