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

Update default regex to not match newlines and improve cleanup Markdown list matching #639

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

baincd
Copy link
Contributor

@baincd baincd commented Jun 2, 2022

I understand that changes to the default regex are rejected. However, this PR does not change the default regex, but rather fixes a couple of issues with the default regex.

Here is the current regex, and the proposed regex as entered in the Settings UI editor
image

  1. In the markdown numbered list match, escape the .
    • The unescaped period matched any character, causing lines like 1X TODO to match
  2. Enforce at least 1 space after - or 1.
    • This will fix lines like 1.TODO or -TODO matching, as those are not valid markdown lists
  3. Change the space before ($TAGS) to only match spaces and tabs (not newlines)

@baincd
Copy link
Contributor Author

baincd commented Nov 6, 2022

Hi @Gruntfuggly , I understand you don't want change the default regex, but I feel this isn't really a change but a fix to the existing default regex. You can see my explanation for my suggested changes above. I have already applied this in my own settings, but I really think this fix could benefit other users and even potentially resolve some outstanding issues.

Either way you decide, thank you for your consideration 😄

@JustOptimize
Copy link

JustOptimize commented Apr 25, 2023

I found an issue with this regex, it is matching tags even if they are not preceded by any comment
An example with the ! tag
image

I was using this regex for a while and it doesn't have this issue but it matches the tag if it is proceeded by anything even if it is not in the list (//|# ...
(//|#|--|;|/\*|^|^[ \t]*(-|\d+.))[^\s]*($TAGS)
image

@baincd
Copy link
Contributor Author

baincd commented Apr 27, 2023

@JustOptimize,

I found an issue with this regex, it is matching tags even if they are not preceded by any comment An example with the ! tag image

I would disagree that this is a bug. I believe this is exactly how it is intended to work - one of the conditions a tag is matched is if the only chars before it on the line is whitespace

To accomplish what I believe you are looking for (to not to match tags preceded by only whitespace), I would suggest trying this modified version of this PR: (//|#|<!--|;|/\\*|^[ \\t]*(-|\\d+\\.)[ \\t])[ \\t]*($TAGS)

Of course you are free to make this modification in your own settings, but this change should not be made to the default regex because as @Gruntfuggly has made perfectly clear the functionality of default regex will not be changed. (This PR only fix bugs in the default regex without changing it's functionality)

@GitMensch
Copy link

The changes 1+2 actually are a change that effects current highlighting "intended for better highlighting markdown", while "the main part" 3 does fix a bug. Can you please move 1+2 out to a separate PR in the hope that the main change gets merged to close the linked bug?

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

Successfully merging this pull request may close these issues.

Wierd behavior in tree view with open file scan
3 participants