Skip to content
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

improve buffer URI format #2362

Closed
wants to merge 1 commit into from
Closed

improve buffer URI format #2362

wants to merge 1 commit into from

Conversation

rchl
Copy link
Member

@rchl rchl commented Nov 12, 2023

Per discussion in #2360, change the URI format for unsaved files so that the part after the scheme is parsed as a path and not as an authority (netloc in python).

Shouldn't really affect anything (neither fix the Pyright issue) but seems like a more correct thing to do.

(Also workarounds the issue with urlparse failing to parse the URI correctly when path starts with numbers - add View- prefix).

@jwortmann
Copy link
Member

Might be better to change the format to buffer:view-<id> or something like that rather than adding a workaround for urllib.

This prefix is also just a workaround for the urllib bug, it's just a different one.

Actually it is the buffer ID, not the view ID. I can push my test changes from yesterday so that we could compare; I think I'd slightly prefer the other (explicit) workaround, and it might improve a few other places in the code too. But I'm not sure yet.

@@ -26,7 +26,7 @@ def filename_to_uri(file_name: str) -> str:
def view_to_uri(view: sublime.View) -> str:
file_name = view.file_name()
if not file_name:
return "buffer://{}".format(view.buffer_id())
return "buffer:View-{}".format(view.buffer_id())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not Buffer- prefix? 🤪

@jwortmann
Copy link
Member

Two reasons why I probably prefer the other approach:

  • The underlying bug is that urllib can't parse the buffer:123 format correctly, and not that the format would be invalid somehow. So I think the fix/workaround should be for the parsing, and not to modify the URI format.
  • We should keep in mind what might be a potential format that could natively be used by ST in the future (even if it might never be added): Implement view.uri() sublimehq/sublime_text#3742
    I assume that buffer:123 should be more likely than buffer:Buffer-123.

@rchl rchl closed this Nov 13, 2023
@rchl rchl deleted the fix/buffer-uri branch November 13, 2023 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants