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

check OS to use appropriate program to open files in rga-fzf #260

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

tandetat
Copy link
Contributor

Replace hardcoding evince to open pdfs and xdg-open to open other files when using rga-fzf use those if the OS is Linux, and use open if the OS is macOs. If the OS is neither macOs nor Linux, it fails to open. I didn’t add an option for Windows because I’m not familiar enough with it to suggest a program.

This is intended to fix #240 and #253 by checking the OS before running xdg-open and evince.

let (cmd, pdf_cmd) = match env::consts::OS {
"macos" => ("open", "open -a Preview.app"), // use native Preview for macOs
"linux" => ("xdg-open", "evince"), // use evince for linux
&_ => ("", ""),
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't personally verified this, but start appears to be relevant command on Windows. Supposedly when passed a file as its sole argument it behaves the same as double-clicking the file in explorer.exe, namely opening the file in the associated default program.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the info! I did some digging around that and apparently start doesn’t work consistently across shells in Windows. This info might be outdated since the comment talking about this is from 2018.

Also, in the doc you sent about start it says you can run non-executable files through their file association by typing the name of the file as a command. A little bit of search led me to this two articles from last year which claim the same thing: howtogeek article, windows loop article.

I might try to personally test these and see which alternative is best with a Windows VM or a Windows Docker container.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, in the doc you sent about start it says you can run non-executable files through their file association by typing the name of the file as a command.

Thinking out loud: I wonder if that is implemented at the level of cmd.exe/PowerShell or somewhere deeper. If the former, it might be easier to look at how other Rust programs are solving this than trying to reason it from first principles…

Copy link
Contributor

Choose a reason for hiding this comment

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

Finally researched this a bit more. It looks like start is a built-in in both cmd.exe and Windows variants of PowerShell, not a standalone tool that can be trivially executed. The standard approach (see CPython's implementation as an example) seems to be to spin out a shell process to run start.

However, it might be worth just pulling in a dependency like open rather than implement this directly. There appear to be some odd edge cases that I'd rather not have to keep track of and handle, e.g. Byron/open-rs#73.

Copy link
Contributor

Choose a reason for hiding this comment

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

That said, I'd personally be in favor of merging this PR as-is and leaving the Windows implementation as a todo. master currently lacks Windows support, so this wouldn't constitute a regression in any way.

@phiresky Any opinions?

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.

Unable to open a searched file on macOS
4 participants