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

feat: open files without file:// protocol #4713

Merged

Conversation

amustaque97
Copy link
Contributor

Overview: add support for file paths starts with ../ ./ and /

To implement this I extended the existing regex variable in the src/config dir. In a few test cases, extra space is added intentionally to verify we don't include space in the URL as it looks & feels odd.

Here is the text file content used for testing

~ cat test.txt
https://google.com
file:///Users/ABC/oss/ghostty/build.zig
file:///Users/ABC/oss/ghostty/debug.sh


/Users/ABC/oss/ghostty/src/../debug.sh
../../Applications/Zed.app/Contents/Info.plist
[link](/home/user/ghostty.user/example) -- non-existent
[link](/Users/ABC/oss/ghostty/src/../debug.sh)
first time ../../Applications/Zed.app/Contents/Info.plist contributor~

Here is the screen recording of the changes.
Screencast

To implement this I extended the existing regex variable in the `src/config` dir. In a few test cases, extra space is added intentionally to verify we don't include space in the URL as it looks & feels odd.
@mitchellh
Copy link
Contributor

Well done. Thanks.

@mitchellh mitchellh merged commit 262c76e into ghostty-org:main Jan 7, 2025
24 checks passed
@github-actions github-actions bot added this to the 1.1.0 milestone Jan 7, 2025
@varyform
Copy link

varyform commented Jan 7, 2025

While it's a good improvement, there are some issues:
↳ app/views/albums/index.html.haml:1
will be recognized as /views/albums/index.html.haml:1

Which is neither a correct path nor a valid file protocol URL (b/c of line number)

I think these kind of things will behave much better once link configuration option is available.

Temp solution for my personal needs (Rails+Zed):
https://github.com/ghostty-org/ghostty/compare/main...varyform:ghostty:extra-protocols?expand=1

module CustomPathLogger
  private def from_rails_root(string)
    "zed://file#{string}"
  end
end

ActiveSupport.on_load(:action_view) do
  ActionView::LogSubscriber.prepend(CustomPathLogger)
end

@mitchellh
Copy link
Contributor

While it's a good improvement, there are some issues:
↳ app/views/albums/index.html.haml:1
will be recognized as /views/albums/index.html.haml:1

Indeed, it's not a perfect solution but it's strictly an improvement. This is a well unit tested file, feel free to add additional test cases and resolve them. That'd be very helpful!

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