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

Suggestion Regarding matchFiles #209

Open
jaswrks opened this issue Sep 27, 2023 · 2 comments
Open

Suggestion Regarding matchFiles #209

jaswrks opened this issue Sep 27, 2023 · 2 comments

Comments

@jaswrks
Copy link

jaswrks commented Sep 27, 2023

The current default is **/*, which is likely a performance issue for projects that have yet to configure it. Additionally, I'm seeing that both workspace.findFiles() in VS Code and also minimatch() are used against the glob patterns configured for Comment Anchors. This results in a few problems:

  • Not possible to configure glob patterns that are relative to the project, because minimatch is used after finding the files with VS Code, and the globbing is then against the full document path. It would be nice if Comment Anchors could match against relative paths.

  • Your implementation of minimatch doesn’t set the { dot: true } option, so it's not possible to create TODO items inside of .dotfiles within a project, unless given explicitly. It would be nice if { dot: true } could be enabled when calling on minimatch in your extension.

  • The use of excludeFiles is limiting as just a string. I see that workspace.findFiles only accepts a string glob pattern, but it would be nice if the additional filtering that you are doing with minimatch could consider an array of secondary patterns to filter out more things at a more granular level, leaving the broad strokes as they are for workspace.findFiles.

@machinemessiah
Copy link

Not possible to configure glob patterns that are relative to the project, because minimatch is used after finding the files with VS Code, and the globbing is then against the full document path. It would be nice if Comment Anchors could match against relative paths.

Spent quite a while banging my head against the wall trying to work out a relative glob pattern only to end up using an absolute path (which also required my drive letter to be lowercase and prefixed with a forward slash), only to come here once I'd got it working to post an issue and see your post.

Wish I'd just come straight here first and saved myself some time!

The current implementation of matchFiles quite simply doesn't work "as expected", or at least when compared to essentially every other extension featuring the same type of setting that I've ever used (not to mention the fact that the description of how said setting functions also gives off the impression that it works how one would "expect", and not in the way it actually does - which is what caused the major headache for me personally).

macjuul pushed a commit that referenced this issue Jan 19, 2024
macjuul pushed a commit that referenced this issue Jan 19, 2024
@macjuul
Copy link
Member

macjuul commented Jan 19, 2024

I have implemented fixes for the first two topics you mentioned and will be available in the next release. Unfortunately I do not have enough time or resources to update the parsing logic in regards to supporting multiple glob patterns (related #69), however I am more than willing to look at a PR updating this behavior.

My Apologies for the long wait!

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

3 participants