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

findTextInFiles fails on redundant glob #104889

Closed
connor4312 opened this issue Aug 18, 2020 · 10 comments
Closed

findTextInFiles fails on redundant glob #104889

connor4312 opened this issue Aug 18, 2020 · 10 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug *duplicate Issue identified as a duplicate of another issue(s) search Search widget and operation issues search-api
Milestone

Comments

@connor4312
Copy link
Member

Refs #104648 (comment)

There's an outfile pattern that we compile to new RelativePattern('<workspaceFolder>', 'dist/**/*.js, dist/*.js'). This fails to match anything, but new RelativePattern('<workspaceFolder>', 'dist/**/*.js') works.

Complete options:

findTextInFiles(
  { pattern: 'sourceMappingURL', isCaseSensitive: true },
  {
    include: {
      base: "E:\\TestBed\\vscode-ts-debug-failure",
      pattern: "dist/**/*.js, dist/*.js",
    },
    exclude: "",
    useDefaultExcludes: false,
    useIgnoreFiles: false,
    useGlobalIgnoreFiles: false,
    followSymlinks: true,
    beforeContext: 0,
    afterContext: 0,
    previewOptions: {
      charsPerLine: 9007199254740991,
      matchLines: 1,
    },
  },
)
@roblourens
Copy link
Member

Should it be wrapped in {}?

@connor4312
Copy link
Member Author

We use new RelativePattern -- this was just the result of me copying the options directly from the debugger https:/microsoft/vscode-js-debug/blob/7dd5107733e7982888c129279c06cf13da6c4f93/src/common/sourceMaps/codeSearchStrategy.ts#L86

@roblourens
Copy link
Member

Yeah but I think that if you want that second argument to be parsed as a list, it needs to be wrapped in curly braces.

@connor4312
Copy link
Member Author

As in {dist/**/*.js, dist/*.js}?

@roblourens
Copy link
Member

Yeah

@connor4312
Copy link
Member Author

Ok. Is that something we should do for all patterns? That is, change

forceForwardSlashes(files.patterns.filter(p => !p.startsWith('!')).join(', ')),

to

'{'+ forceForwardSlashes(files.patterns.filter(p => !p.startsWith('!')).join(', ')) + '}'

@roblourens
Copy link
Member

roblourens commented Aug 23, 2020

I believe so. Are they working as is?

@connor4312
Copy link
Member Author

Yea, it works in general, just not this case. I'll give it a shot.

@connor4312
Copy link
Member Author

It seems like ripgrep doesn't like that, {**/*.js, out/*.js} and {**/*.js} don't match any files

@connor4312
Copy link
Member Author

This also seems to fail if the pattern base is a workspace folder and the pattern navigates outside of it (e.g. ../foo/**/*.js)

@roblourens roblourens added the search Search widget and operation issues label Dec 13, 2022
@connor4312 connor4312 added the *duplicate Issue identified as a duplicate of another issue(s) label Dec 14, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jan 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug *duplicate Issue identified as a duplicate of another issue(s) search Search widget and operation issues search-api
Projects
None yet
Development

No branches or pull requests

2 participants