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

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

Open
agowa opened this issue Feb 14, 2024 · 0 comments
Open

Comments

@agowa
Copy link

agowa commented Feb 14, 2024

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).

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

No branches or pull requests

1 participant