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: Behavior between inline and multiline comments inconsistent #454

Open
aarondill opened this issue Dec 6, 2022 · 1 comment · May be fixed by #460
Open

Javascript: Behavior between inline and multiline comments inconsistent #454

aarondill opened this issue Dec 6, 2022 · 1 comment · May be fixed by #460

Comments

@aarondill
Copy link

Issue:

Multiline and inline comments have seemingly opposite behaviors when choosing styles with multiple tags.

Expected outcome:

Coloring of the two situations to be the same if the same tags are used.

Actual outcome:

Multiline comments use the first tag found to decide the styling, while inline comments chose the last tag found before a whitespace.

Environment:

Extension Version(Last Updated): 11/1/2022, 11:37:57
Vscode Extension Version: 1.73.1 on Debian Linux

Steps to reproduce:

Copy the following code into an editor with the extension activated:

/*
*! This is green
TODO? This is orange
*/
//*! This is red
//TODO? This is blue

Further details:

The preferred behavior, in my opinion, would be the behavior shown by multiline comments. Inline comments currently provide unexpected behavior, i.e. tags such as //TODO!, where a tag after another overwrites the first tag; while multiline comments provide more expected behavior, i.e. the first tag decides the comment styling.

@aarondill
Copy link
Author

aarondill commented Dec 6, 2022

Taking a quick look through /src/parser.ts, it seems like this line in parser.SetRegex is the issue.

// parser.SetRegex;
this.expression += "(";
this.expression += characters.join("|");
this.expression += ")+(.*)";

The key issue I notice is the plus after the character list in SetRegex, matching all sequential tags, but only the first needs to be matched within the first group.

Using this function, you can see the difference with and without the operator.

function match(string, update) {
    let expression = "(\/\/)+( |\t)*"
    const characters=["\\*", "\\$"]
    expression += "(" + characters.join("|") + ")"
    if(!update) expression+="+"
    expression+="(.*)";
    const regexFlags = "ig";
    const regEx = new RegExp(expression, regexFlags);
    const match = regEx.exec(string);
    console.log(`regex used: /${expression}/${regexFlags}`)
    console.log(`tag found: ${match[3]}`)
    return match
}
match("//*$hello", false) //tag found: $
match("//*$hello", true) //tag found: *

as you can see, without the plus, it matches just the first tag that occurs, matching any other characters(including other tags) in the 4th capture group.

Unless there was a particular reason(perhaps language compatibility?) that I am unaware about, it seems this is a mistake and should be removed to allow expected behavior.

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