🐛 Update GetErrorsTool to handle Windows drive letter casing#320053
Open
Wangmz-1211 wants to merge 4 commits into
Open
🐛 Update GetErrorsTool to handle Windows drive letter casing#320053Wangmz-1211 wants to merge 4 commits into
Wangmz-1211 wants to merge 4 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates GetErrorsTool path/URI matching to correctly associate diagnostics on Windows when the drive letter casing differs, and adds regression tests for both direct diagnostics lookup and full tool invocation output.
Changes:
- Switch URI parent/equality checks to
extUriBiasedIgnorePathCaseto tolerate case differences in Windows paths. - Add Windows-only tests validating diagnostics matching and tool output when drive letter casing differs.
- Extend the test workspace setup with a Windows-drive URI document for the new scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| extensions/copilot/src/extension/tools/node/getErrorsTool.tsx | Adjusts URI equality/parent checks to be case-insensitive (biased) so Windows drive-letter casing mismatches still match diagnostics. |
| extensions/copilot/src/extension/tools/node/test/getErrorsTool.spec.tsx | Adds Windows-specific regression tests and registers a Windows-drive document in the test workspace. |
Comment on lines
+145
to
+158
| test.skipIf(!isWindows)('getDiagnostics - matches Windows file path with different drive letter casing', () => { | ||
| const diagnostic = { | ||
| message: 'Drive letter casing diagnostic', | ||
| range: new Range(0, 6, 0, 11), | ||
| severity: DiagnosticSeverity.Error | ||
| }; | ||
| diagnosticsService.setDiagnostics(upperCaseDriveFile, [diagnostic]); | ||
|
|
||
| const results = tool.getDiagnostics([{ uri: lowerCaseDriveFile, range: undefined }]); | ||
|
|
||
| expect(results).toEqual([ | ||
| { uri: upperCaseDriveFile, diagnostics: [diagnostic] } | ||
| ]); | ||
| }); |
Comment on lines
+56
to
+58
| const upperCaseDriveDoc = createTextDocumentData(upperCaseDriveFile, 'const drive = "C";\n', 'ts').document; | ||
|
|
||
| collection.define(IWorkspaceService, new SyncDescriptor(TestWorkspaceService, [[workspaceFolder], [tsDoc1, tsDoc2, jsDoc, noErrorDoc, eslintErrorDoc, emptyLineErrorDoc]])); | ||
| collection.define(IWorkspaceService, new SyncDescriptor(TestWorkspaceService, [[workspaceFolder], [tsDoc1, tsDoc2, jsDoc, noErrorDoc, eslintErrorDoc, emptyLineErrorDoc, upperCaseDriveDoc]])); |
| import { Disposable } from '../../../util/vs/base/common/lifecycle'; | ||
| import { ResourceSet } from '../../../util/vs/base/common/map'; | ||
| import { isEqualOrParent } from '../../../util/vs/base/common/resources'; | ||
| import { extUriBiasedIgnorePathCase } from '../../../util/vs/base/common/resources'; |
| for (const path of nonNotebookPaths) { | ||
| // we support file or folder paths | ||
| if (isEqualOrParent(resource, path.uri)) { | ||
| if (extUriBiasedIgnorePathCase.isEqualOrParent(resource, path.uri)) { |
Comment on lines
+104
to
+107
| const isExactMatch = extUriBiasedIgnorePathCase.isEqual( | ||
| resource, | ||
| path.uri, | ||
| ); |
Author
|
@microsoft-github-policy-service agree |
Member
|
I disagree that we should always ignore the path casing |
Author
A new function The new function works for Windows only, and change only the drive letter to lowercase. This avoids always ignoring path casing. |
Wangmz-1211
force-pushed
the
fix/getErrors-tool-uri-parsing
branch
2 times, most recently
from
20b1d16 to
c748769
Compare
…I matching (Issue microsoft#319858) Fix with an explicit normalizeDriveLetter() that lower-cases only the drive letter component (e.g. C: → c:) before comparison, then uses the case-sensitive isEqual/isEqualOrParent helpers. This matches the Windows semantics where the drive letter is case-insensitive but the rest of the path may not be (see microsoft#319858). Add a dedicated unit-test suite covering all branches of normalizeDriveLetter: non-Windows identity, uppercase/lowercase drive letters, path components beyond the drive, short paths, root paths, UNC-style paths, and non-letter characters at the drive position.
Wangmz-1211
force-pushed
the
fix/getErrors-tool-uri-parsing
branch
from
c748769 to
cd65ddf
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #319858
This pull request updates the
GetErrorsToolto improve handling of file path comparisons on Windows, specifically addressing differences in drive letter casing. The main changes ensure that file and folder path matching is case-insensitive where appropriate, and new tests are added to verify this behavior.Path Comparison Improvements:
Updated
getErrorsTool.tsxto useextUriBiasedIgnorePathCase.isEqualOrParentandisEqualfor path comparisons, ensuring case-insensitive matching of file and folder paths, which is especially important on Windows systems.Testing Enhancements:
Added Windows-specific test cases in
getErrorsTool.spec.tsxto verify that diagnostics are correctly matched even when the drive letter casing differs (e.g.,C:vsc:). These tests are conditionally run only on Windows.These changes improve cross-platform reliability and correctness of diagnostic retrieval in the Copilot extension.