Fix opening file with colon #17281
Merged
Fix opening file with colon #17281
Conversation
This file contains 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
|
We require contributors to sign our Contributor License Agreement, and we don't have @fantasticfears on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'. |
Note: 1. POSIX path supports `:` or `.` as filename 2. zed use `PathWithPosition` for parsing CLI path and in-editor navigation. Assumptions and implementation: 1. I assume that `PathWithPosition` doesn't know if filepath exists or not. Users of the class should be mindful. 2. If a filepath has `:`, and position tag is not well-formatted, the implementation will match from right to left to interprete tags. This could make unpredictable behavior. Again, I assume that users of the class should be mindful.
|
@cla-bot check |
|
We require contributors to sign our Contributor License Agreement, and we don't have @fantasticfears on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'. |
|
The cla-bot has been summoned, and re-checked this pull request! |
cla-bot
bot
added
the
cla-signed
jvmncs
approved these changes
Sep 17, 2024
noaccOS
pushed a commit
to noaccOS/zed
that referenced
this pull request
Oct 19, 2024
Closes zed-industries#14100 Release Notes: - Fixed unable to open file with a colon from Zed CLI ----- I didn't make change to tests for the first two commits. I changed them to easily find offending test cases. Behavior changes are in last commit message. In the last commit, I changed how `PathWithPosition` should intreprete file paths. If my assumptions are off, please advise so that I can make another approach. I also believe further constraints would be better for `PathWithPosition`'s intention. But people can make future improvements to `PathWithPosition`.
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.
Closes #14100
Release Notes:
I didn't make change to tests for the first two commits. I changed them to easily find offending test cases. Behavior changes are in last commit message.
In the last commit, I changed how
PathWithPositionshould intreprete file paths. If my assumptions are off, please advise so that I can make another approach.I also believe further constraints would be better for
PathWithPosition's intention. But people can make future improvements toPathWithPosition.