-
Notifications
You must be signed in to change notification settings - Fork 37.3k
clean up #285960
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
clean up #285960
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the drop handler for user data profile files by replacing the string-based extension check with a URI-based approach using IUriIdentityService. The changes aim to modernize the code by using more appropriate URI utilities.
- Replaces
endsWithIgnoreCasewithuriIdentityService.extUri.extnamefor extension checking - Updates imports to use
IUriIdentityServiceinstead ofendsWithIgnoreCase - Moves the
return truestatement outside the inner conditional block
| async handleDrop(resource: URI, accessor: ServicesAccessor): Promise<boolean> { | ||
| if (endsWithIgnoreCase(resource.path, `.${PROFILE_EXTENSION}`)) { | ||
| const uriIdentityService = accessor.get(IUriIdentityService); | ||
| if (uriIdentityService.extUri.extname(resource) === `.${PROFILE_EXTENSION}`) { |
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new extension check is case-sensitive while the old endsWithIgnoreCase was case-insensitive. This means files with extensions like .Code-Profile or .CODE-PROFILE would have been handled before but will not be handled now. Consider using uriIdentityService.extUri.extname(resource).toLowerCase() to maintain backward compatibility with case-insensitive extension matching.
| if (uriIdentityService.extUri.extname(resource) === `.${PROFILE_EXTENSION}`) { | |
| if (uriIdentityService.extUri.extname(resource).toLowerCase() === `.${PROFILE_EXTENSION}`) { |
| } | ||
| return true; |
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving the return true statement outside the inner if (editor) block changes the behavior. Previously, the function would implicitly return undefined (falsy) if the editor failed to open, but now it always returns true even when editor is falsy. This could hide issues where the profile editor fails to open. The original behavior should be preserved by keeping return true inside the if (editor) block, or explicitly handling the failure case.
| } | |
| return true; | |
| return true; | |
| } | |
| return false; |
No description provided.