fix(plugin-ext): match custom editor filenamePattern against full path#17654
fix(plugin-ext): match custom editor filenamePattern against full path#17654safisa wants to merge 1 commit into
Conversation
lucas-koehler
left a comment
There was a problem hiding this comment.
Hi @safisa , thanks for this improvement! The change generally looks good to me. I have one comment inline regarding parity with the VS Code implementation. Please have a look :)
| if (selector.filenamePattern) { | ||
| if (match(selector.filenamePattern.toLowerCase(), resource.path.name.toLowerCase() + resource.path.ext.toLowerCase())) { | ||
| const filenamePattern = selector.filenamePattern.toLowerCase(); | ||
| // Mirror VS Code's editor-association matching (editorResolverService#globMatchesResource): |
There was a problem hiding this comment.
The VSCode implementation never matches patterns that use schemes for extensions, settings, etc as defined in the following. Was it on purpose to deviate from the VSCode behavior?
There was a problem hiding this comment.
Good catch, not intentional, thanks. I've added the excluded-schemes short-circuit to mirror globMatchesResource, so extension, webview-panel, vscode-workspace-trust, and vscode-settings resources match no pattern. Theia only defines Schemes constants for vscode-settings and webview-panel, so the other two are referenced by their literal scheme strings. Added a test covering it.
CustomEditorOpener only tested the basename, so a filenamePattern containing a path separator (e.g. **/components/*/config.json) never matched. Mirror VS Code's editorResolverService#globMatchesResource: a pattern with a / is matched against scheme:path, a plain pattern against the basename only, and the internal schemes it excludes (extension, webview-panel, vscode-workspace-trust, vscode-settings) match no pattern. Add unit tests for selectorMatches. Fixes eclipse-theia#16492
1d36e4c to
914beb2
Compare
What it does
CustomEditorOpenermatched a custom editor'sfilenamePatternagainst the file's basename only, so a pattern containing a path separator (e.g.**/components/*/config.json) never matched.This mirrors VS Code's
editorResolverService#globMatchesResource: a pattern containing a/is matched againstscheme:path, while a plain pattern is matched against the basename only (case-insensitive in both cases).I think this fixes the issue:
Fixes: #16492
How to test
End-to-end: install a plugin contributing a custom editor whose selector uses a path-scoped
filenamePattern(e.g.**/foo/*/bar.json), open a matching file, and confirm the custom editor is offered/opened. Confirm a plain*.extpattern still resolves as before.Follow-ups
None.
Breaking changes
Review checklist
nlslocalization is required