-
Notifications
You must be signed in to change notification settings - Fork 165
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
Add OpenExternal
, a function for opening files in the OS
#4295
Conversation
d76d66c
to
ec3747e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this feature, and I think OpenExternal
is a decent name for it.
3c0f490
to
1979cf2
Compare
I'd be happy to merge this, I just wanted to sanity check before doing so:
|
I don't see a good way to test this automatically in our CI. So I'd just merge this as-is, but I'll wait to give a chance for more comments. |
InstallGlobalFunction( OpenExternal, function(filename) | ||
local file; | ||
if ARCH_IS_MAC_OS_X() then | ||
Exec(Concatenation("open \"",filename,"\"")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I don't want to whine, but I have to repeat one of my pet mantras, namely that "Exec is a really bad function, we should never have added it like that". Besides not even allowing to check the return value, it also forces us to do silly quoting tricks like the above; and this is not even complete, a filename with a "
in it will break it. We really should think about adding an alternative to Exec
which is a bit better (doesn't go through a shell and hence has no quoting issues, and gives access to return value, and optionally allows for suppressing or redirecting output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(But of course none of this is required for this PR!)
OpenExternal
, a function for opening files in the OS
Add a function "OpenExternal", which opens files in other apps in the file system. While I like the names used by existing packages (like Splash), I also don't want to clash with those existing packages.
Rather than base this on the existing code used for help, I wrote a new function. This does remove the customizability, but also means we can open any file type. Also, in my experience, nowadays the "default" operating system method of opening files is what I want.
This also does not implement the functionality provided by (for example) semigroups, where files such as '.dot' and '.tex' files are compiled before being opened.
I'm very happy for suggestions on naming / functionality / etc. I thought having a PR would give a place to discuss such issues.
Discussed in #4272