search

Gruntfuggly / todo-tree

Use ripgrep to find TODO tags and display the results in a tree view
Other
1.47k stars 142 forks source link

Default`todo-tree.regex.regex` value may be wrong #341

Closed munierujp closed 4 years ago

munierujp commented 4 years ago

Default todo-tree.regex.regex value:

((//|#|<!--|;|/\*|^)\s*($TAGS)|^\s*- \[ \])

)

Perhaps the correct value would be:

((//|#|<!--|;|/\*|\^)\s*($TAGS)|^\s*- \[ \])

)

(It's escaping ^ symbol)

or...:

(^(//|#|<!--|;|/\*)\s*($TAGS)|^\s*- \[ \])

)

(It's using ^ as regex's metacharacter which indicates starting position)

Gruntfuggly commented 4 years ago

The default regex is intended to match the start of the line or the comment characters, so I think it is correct. It definitely shouldn't be escaped as it isn't intended to match a literal ^.

munierujp commented 4 years ago

@Gruntfuggly

The default regex is intended to match the start of the line or the comment characters

It seems to be different from the description in the README.

This defines the regex used to locate TODOs. By default, it searches for tags in comments starting with //, #, ;, <!-- or /*.

Which is correct? If the README is correct, you can use the following value probably.

(^\s*(//|#|<!--|;|/\*)\s*($TAGS)|^\s*- \[ \])

)

This works correctly like this:

Gruntfuggly commented 4 years ago

The README doesn't describe the entire regex, it just gives an outline of what it does. The regex already matches your two examples, so I don't understand what the issue is?

munierujp commented 4 years ago

@Gruntfuggly When todo-tree.regex.regex value is ((//|#|<!--|;|/\*|^)\s*($TAGS)|^\s*- \[ \]), it matches three lines (include 4th line). Because 4th line starts with spaces and tag name (TODO). And 4th line is just prop name, not TODO comment.

Gruntfuggly commented 4 years ago

OK - I understand now. I don't think this is necessarily wrong, it just doesn't work for this case. I would expect users would need to tweak the regex to make it work for them anyway - it is impossible to make a default that works perfectly for everybody.

I don't really want to change the default as there are many users who do still use the default, and when I have changed it in the past it causes expected problems for people.

As long as you can configure it to work for yourself, I would prefer to leave it.

munierujp commented 4 years ago

@Gruntfuggly

I don't really want to change the default as there are many users who do still use the default, and when I have changed it in the past it causes expected problems for people.

I see. Thank you 😎