Max Nikulin writes: > However, in my opinion, it does not address original Craig's issue. The > patch improves handling of *non-text* files requiring *external* > viewers, while Craig complained that behavior for shell script is > incorrect and his problem is tightly bound to erased `mailcap-mime-data'. Ideally, we need a feedback from him. For emacs-27 specifically, we might need to work around the bug. However, I am not sure what would be the best way to do it. The easiest can be changing the default value of org-file-apps-gnu on Emacs 27 specifically to not use mailcap at all. But I am pretty sure that we can do better. > I can not reproduce behavior he observed exactly, Org does not opens > shell scripts by less, but it tries and silently (it is expected) fails. > > My results (emacs-27): > 1. If there is no mailcap files at all, the script is opened > in the same emacs session that is correct from my point of view. > 2. If I add ~/.mailcap > text/plain; less '%s'; needsterminal > then I get silent failures > Running less /etc/profile...done > 3. With your patch and the following additional entry in ~/.mailcap > (notice "text" instead of "application" and just "emacs") > text/x-shellscript; emacs %s; test=test -n "$DISPLAY" > new Emacs session is started. It is a problem but partially > it is caused by incorrect mailcap configuration. > Unlike "text/plain" that would be handled by view-mode > unless `mailcap-mime-data' were erased in Emacs-27, > "text/x-shellscript" is handled by less on my main system > due to mailcap while I would expect same Emacs session. I am confused here. org-file-apps-gnu says that we rely on mailcap: ((remote . emacs) (system . mailcap) (t . mailcap)) So, is (3) following what you would expect from mailcap (regardless whether it is incorrectly configured or not)? Wrong configuration of mailcap is none of Org business - we need not to be "smart" and fix user "errors". They may be deliberate. > I read implementation of `org-open-file' once more and now I see that > currently remote files can not be processed by mailcap code path even > with custom `org-file-apps', so thank you for explanation. In some cases > it may be handy to launch remote viewer from a host accessed through > ssh, but let's leave it aside. This is exactly why you can always customize org-file-apps. >> - (let* ((mime-type (mailcap-extension-to-mime (or ext ""))) >> + (let* ((mime-type (if (executable-find "file") > > I would consider (and ... (not remp)) despite currently it is redundant. > Just to mitigate consequences if other parts of this complicated > function will be modified. On the other hand > `start-process-shell-command' is not ready for remote files anyway, so > major cleanup would be required. I would be in favor of a cleanup (by someoneā„¢), but I am against redundancy. Such redundancy may mask bugs making them difficult to debug. Not to mention code readability. >> + (shell-command-to-string >> + (format "%s --brief --mime-type %s" >> + (executable-find "file") >> + (shell-quote-argument (convert-standard-filename file)))) > > It is not enough to cure my paranoia. However the following case is > rather pathological > > mkdir 'Program Files' > ln -s /usr/bin/file 'Program Files'/ > PATH="$HOME/Program Files:$PATH" \ > emacs -Q -L ~/src/org-mode/lisp/ ~/examples/org/open-script.org & > > Debugger entered--Lisp error: (wrong-type-argument stringp nil) > string-match("/" nil 0) > split-string(nil "/") > mailcap-mime-info("/bin/bash: line 1: /home/ubuntu/Program: No such > f...") > (let* ((mime-type (if (executable-find "file") > (shell-command-to-string (format "%s --brief --mime-type %s" > (executable-find "file") (shell-quote-argument > (convert-standard-filename file)))) (mailcap-extension-to-mime (or ext > "")))) (command (mailcap-mime-info mime-type))) (if (stringp command) > (setq cmd command) (setq cmd 'emacs))) Well. If we want to be this paranoid, could you write a generic safe shell-command wrapper that takes care of various edge cases? Then, we can add that wrapper to org-macs and reuse it in various places where we need to run external command. > Another corner case: > > file --brief --mime-type tstorg-sh-symlink > inode/symlink > file --brief --mime-type --dereference tstorg-sh-symlink > text/x-shellscript I added the extra argument as you suggested. See the new version of the patch. Though my man tells me that --dereference is the default. Not on your system apparently. >> + (executable-find "file") >> + (shell-quote-argument (convert-standard-filename file)))) >> + (mailcap-extension-to-mime (or ext "")))) > > Actually MIME type for shell scripts varies a lot > > (mailcap-extension-to-mime "sh") => "text/x-sh" > > run-mailcap --norun examples/org/script/tstorg.sh > Error: no "view" mailcap rules found for type "application/x-sh" > > And "text/x-shellscript" as above. This should not matter for us. As long as mailcap-mime-info returns something meaningful, we should be good to go. Unless mailcap-mime-info itself is buggy. Best, Ihor