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

fix: better tag selection #2911

Closed
wants to merge 1 commit into from
Closed

fix: better tag selection #2911

wants to merge 1 commit into from

Conversation

Chunjee
Copy link

@Chunjee Chunjee commented May 21, 2021

No description provided.

@github-actions
Copy link

No JS Changes

Generated by 🚫 dangerJS against 522c9d3

@RunDevelopment
Copy link
Member

@Chunjee Could you please explain what issue you are trying to fix and how your fix works?

@Chunjee
Copy link
Author

Chunjee commented May 24, 2021

Incorrect tag selection (first red box):
image

Post fix:
image

@RunDevelopment
Copy link
Member

I see, in that case, your fix is not correct. Try this example.

I have a question: are labels line-based? So can there be any no-space, no-comments after the label on the same line?

If they are, then your problem can be fixed like this:

@@ -11,8 +11,13 @@ Prism.languages.autohotkey = {
                }
        ],
+       'tag': {
+               // labels
+               pattern: /^([ \t]*)[^\s,`":]+(?=:[ \t]*$)/m,
+               lookbehind: true
+       },
        'string': /"(?:[^"\n\r]|"")*"/m,
-       'tag': /^[ \t]*[^\s:]+?(?=:(?:[^:]|$))/m, //labels
        'variable': /%\w+%/,
        'number': /\b0x[\dA-Fa-f]+\b|(?:\b\d+(?:\.\d*)?|\B\.\d+)(?:[Ee]-?\d+)?/,

@Chunjee
Copy link
Author

Chunjee commented May 25, 2021

I am afraid that comments are allowed after a label; and in some unique circumstances, before.

After:

validLabel: ; a comment

Before:

/* multiline comment
*/ validLabel:

@RunDevelopment
Copy link
Member

comments are allowed after a label; and in some unique circumstances, before.

Perfect. In that case, you can use the code I commented above.

@Chunjee
Copy link
Author

Chunjee commented May 25, 2021

Using the code in the comments appears to have a similar issue:
image

@RunDevelopment
Copy link
Member

No, it doesn't.

image

You probably forgot to remove the old tag (see diff above).

@Chunjee
Copy link
Author

Chunjee commented May 25, 2021

Thank you, after some more experimentation I think the issue was a duplicate string definition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants