wayou / vscode-todo-highlight

a vscode extension to highlighting todos, fixmes, and any annotations...
https://marketplace.visualstudio.com/items?itemName=wayou.vscode-todo-highlight
MIT License
509 stars 98 forks source link

Error in example keywordsPattern and keywords mutually exclusive or bug? #237

Open agowa opened 8 months ago

agowa commented 8 months ago

Hi, using keywordsPattern together with keywords as in the provided config example doesn't work.

If this is intentional

Could you please add a comment to the example that keywordsPattern and keywords are mutually exclusive and that keywordsPattern will take precedence?

If this is a bug or limitation

If this is a bug or limitation and both should work together, where as something that matches using keywordsPattern but is not also in keywords will use the defaultStyle.

The implementation could be something as simple as joining the pattern from the keywords with the regex e.g. by adding () around the value provided by keywordsPattern and OR-joining it with all of the pattern provided within keywords (with each of them also wrapped in parantheses to mark them as a group for our actual regex and allow the user to use e.g. ^ something.*$ within it. ($keywordsPattern)|($keywords[0].name)|($keywords[1].name)

This also would enable using regex pattern within keywords (if it isn't supported already, I didn't check). Either way it would match user expectations of providing both keywordsPattern and keywords simultaneously.

(TODO:|FIXME:|\\(([^)]+)\\))|(DEBUG:)|(REVIEW:)|(NOTE:)|(HACK:)|(TODO:) would be the internal lookup regex for the example config in this case. It's not nice and has redundancies, however it is easy to generate from the configuration and the impact on performance should be negligible because a) the library/engine will be able to optimize it and b) even if no optimizations are applied by the engine it should still match usual performance expectations (and for edge cases with very large projects and/or files a user can always provide their own regex and do a match that doesn't apply any formatting to avoid further evaluations, the only downside is that that probably would require providing an empty defaultStyle and cnp-ing the config into all desired matches. However as that only is relevant in very extreme edge cases we probably can just ignore it (for now).)

For reference the relevant part of the example config:

{
    // ...
    "todohighlight.keywords": [
        "DEBUG:",
        "REVIEW:",
        {
            "text": "NOTE:",
            // ...
        },
        {
            "text": "HACK:",
            // ...
        },
        {
            "text": "TODO:",
            // ...
        }
    ],
    "todohighlight.keywordsPattern": "TODO:|FIXME:|\\(([^)]+)\\)",
    "todohighlight.defaultStyle": {
    // ...
    }
}

Edit

Appears to be intentional. I just noticed that there is a note within the options table (but not within the example that currently implies that both can be used simultaneously):

NOTE that if this presents, todohighlight.keywords will be ignored. And REMEMBER to escapse the back slash if there's any in your regexp (using \ instead of signle back slash).