Fix drag and drop escaping#18965
Conversation
Summary of ChangesHello @scidomino, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's ability to process file paths from drag-and-drop operations, ensuring full compatibility across Windows, POSIX, and Mac systems. The changes standardize how file paths are interpreted and escaped, leading to a more reliable and consistent user experience when interacting with file references, regardless of the operating system or the specific escaping style used for the path. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
/gemini review |
|
Size Change: -171 B (0%) Total Size: 24.4 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Code Review
This pull request introduces significant improvements to file path parsing for drag-and-drop functionality, aiming to provide robust support across Windows, POSIX, and macOS. The changes include refactoring path splitting and escaping logic into more centralized and platform-aware utilities, along with an expansion of test coverage to validate these new behaviors. While the direction is excellent, I've identified a critical regression in the POSIX path escaping logic that could lead to incorrect behavior, a bug in parsing complex quoted paths, and a minor issue in the test setup that could lead to flaky tests. Addressing these points will help ensure the new implementation is as robust as intended.
There was a problem hiding this comment.
Code Review
This pull request significantly improves the handling of drag-and-drop file paths by introducing more robust, platform-aware parsing and escaping logic for both POSIX and Windows. The changes refactor path utilities and update regular expressions to correctly handle various path formats, including quoted paths. The accompanying tests have been greatly expanded, which is excellent. I found one critical issue in a test file that will cause the build to fail, but otherwise, the changes are solid.
8246b4a to
4b16549
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request significantly improves the handling of file path escaping and parsing for drag-and-drop functionality, making it more robust across both Windows and POSIX environments. The changes introduce platform-specific escaping logic and a more sophisticated parser for various path formats. The test suite has also been substantially enhanced to cover these new scenarios. I've identified one potential issue in the new path parser where it doesn't correctly handle escaped characters within double-quoted strings on POSIX systems, which could lead to incorrect path parsing in some edge cases.
4b16549 to
9636199
Compare
| 'path\\\\\\\\with\\ space.txt', | ||
| ); | ||
| }); | ||
| const mockPlatform = (platform: string) => { |
There was a problem hiding this comment.
nit: as a followup can you cleanup these and other calls to mock the platform with a standard helper util used by all tests in Gemini CLI that mock the platform.
9636199 to
2561109
Compare
|
✅ Patch workflow(s) dispatched successfully! 📋 Details:
🔗 Track Progress: |
|
@bugdotexe very clever, but it won't work without someone with the right permissions approving the github action. |

Summary
Fixes drag-and-drop parsing to fully support windows and POSIX
Details
Drag-and-drop file paths are escaped in several different styles which were previously not always supported:
This PR allows us to accept pretty much anything we get.
This also changes our code to use a consistent OS-specific escaping for at-paths in the text:
Also, to reduce the aggressiveness of at-prefixing, we will only prefix at-paths if all the paths in a paste are valid.
Related Issues
For #17135
How to Validate
drag-and-drop file paths in mac, linux, and windows.
Pre-Merge Checklist