From: Ihor Radchenko <yantar92@gmail.com>
To: Max Nikulin <manikulin@gmail.com>
Cc: emacs-orgmode@gnu.org
Subject: [PATCH v3] Re: Bug in 9.5.3 org--file-default-apps
Date: Mon, 30 May 2022 22:00:27 +0800 [thread overview]
Message-ID: <8735gr15ok.fsf@localhost> (raw)
In-Reply-To: <t6v2ib$mft$1@ciao.gmane.io>
[-- Attachment #1: Type: text/plain, Size: 5563 bytes --]
Max Nikulin <manikulin@gmail.com> 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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v3-0001-org-open-file-Use-file-command-to-determine-mime-.patch --]
[-- Type: text/x-patch, Size: 1586 bytes --]
From 3dac7d62b7c85bde31d27ef48569df9d07ae6eec Mon Sep 17 00:00:00 2001
Message-Id: <3dac7d62b7c85bde31d27ef48569df9d07ae6eec.1653918246.git.yantar92@gmail.com>
From: Ihor Radchenko <yantar92@gmail.com>
Date: Sun, 22 May 2022 12:04:35 +0800
Subject: [PATCH v3] org-open-file: Use file command to determine mime type,
when available
* lisp/org.el (org-open-file): Prefer file command to determine file
type instead of relying on `mailcap-extension-to-mime'. Only fallback
to the latter when file command is not available on the system.
Fixes https://list.orgmode.org/874k1n2hpv.fsf@localhost/T/#mcc10df4ea7937360671e6dea8061153dee518807
---
lisp/org.el | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lisp/org.el b/lisp/org.el
index 95dff27ad..b9a7b4b2e 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -7981,7 +7981,12 @@ (defun org-open-file (path &optional in-emacs line search)
(when (eq cmd 'mailcap)
(require 'mailcap)
(mailcap-parse-mailcaps)
- (let* ((mime-type (mailcap-extension-to-mime (or ext "")))
+ (let* ((mime-type (if (executable-find "file")
+ (shell-command-to-string
+ (format "%s --brief --mime-type --dereference %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)
--
2.35.1
next prev parent reply other threads:[~2022-05-30 14:11 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-15 16:49 Bug in 9.5.3 org--file-default-apps Craig STCR
2022-05-16 4:53 ` Ihor Radchenko
[not found] ` <6615610d-93ae-171f-b554-3f4cc79354cc@gmail.com>
2022-05-16 8:33 ` Ihor Radchenko
[not found] ` <86692975-4d5f-6933-3227-c6b208f76862@gmail.com>
2022-05-16 11:57 ` Ihor Radchenko
2022-05-16 15:14 ` Max Nikulin
2022-05-16 19:15 ` Craig STCR
2022-05-16 19:29 ` Craig STCR
2022-05-17 16:03 ` Max Nikulin
2022-05-18 11:36 ` Ihor Radchenko
2022-05-18 16:10 ` Max Nikulin
2022-05-19 13:39 ` Ihor Radchenko
2022-05-19 14:45 ` Max Nikulin
2022-05-20 8:22 ` Ihor Radchenko
2022-05-20 11:31 ` Max Nikulin
2022-05-21 1:44 ` Ihor Radchenko
2022-05-21 14:56 ` Max Nikulin
2022-05-22 4:10 ` [PATCH] " Ihor Radchenko
2022-05-22 7:40 ` Max Nikulin
2022-05-26 4:23 ` [PATCH v2] " Ihor Radchenko
2022-05-29 6:07 ` Max Nikulin
2022-05-30 14:00 ` Ihor Radchenko [this message]
2022-05-30 15:38 ` [PATCH v3] " Max Nikulin
2022-05-31 15:07 ` Max Nikulin
2022-06-04 13:42 ` [DISCUSSION, default settings] Using mailcap as default handler for opening file links (was: [PATCH v3] Re: Bug in 9.5.3 org--file-default-apps) Ihor Radchenko
2022-06-04 17:01 ` Greg Minshall
2022-06-06 12:50 ` [DISCUSSION, default settings] Using mailcap as default handler for opening file links Max Nikulin
2022-06-06 15:22 ` Max Nikulin
2022-06-06 17:47 ` Bhavin Gandhi
2022-06-10 16:38 ` Max Nikulin
2024-02-12 12:36 ` Ihor Radchenko
2024-02-13 10:59 ` Max Nikulin
2024-02-13 11:27 ` Ihor Radchenko
2024-02-13 15:44 ` Max Nikulin
2024-02-13 15:47 ` Max Nikulin
2024-02-14 14:51 ` Ihor Radchenko
2024-02-27 10:43 ` Max Nikulin
2022-05-29 7:07 ` [PATCH v2] Re: Bug in 9.5.3 org--file-default-apps Max Nikulin
2022-05-23 12:40 ` Craig STCR
2022-05-23 12:59 ` Craig STCR
2022-05-23 14:14 ` Craig STCR
2022-05-23 14:32 ` Craig STCR
2022-05-25 6:18 ` Ihor Radchenko
2022-05-23 15:28 ` Max Nikulin
2022-05-25 6:24 ` Ihor Radchenko
2022-05-25 11:38 ` Craig STCR
2022-05-25 6:10 ` Ihor Radchenko
[not found] ` <e9b4e88d-3807-9080-fa86-c297b17794cb@gmail.com>
2022-05-16 12:38 ` Craig STCR
2022-05-18 11:38 ` Ihor Radchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8735gr15ok.fsf@localhost \
--to=yantar92@gmail.com \
--cc=emacs-orgmode@gnu.org \
--cc=manikulin@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).