emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* org-pdfview-open doesn't work anymore
@ 2016-02-05  5:46 Julien Cubizolles
  2016-02-05  8:33 ` Michael Brand
  0 siblings, 1 reply; 12+ messages in thread
From: Julien Cubizolles @ 2016-02-05  5:46 UTC (permalink / raw)
  To: emacs-orgmode

I've been using org-pdfview (from
https://github.com/markus1189/org-pdfview) to have org-mode open pdf
files generated during export.

--8<---------------cut here---------------start------------->8---
(pdf-tools-install)
(eval-after-load 'org '(progn (require 'org-pdfview)
			      (add-to-list 'org-file-apps '("\\.pdf\\'" . org-pdfview-open))
			      ))
--8<---------------cut here---------------end--------------->8---

Since a recent upgrade, this fails with:

--8<---------------cut here---------------start------------->8---
(wrong-number-of-arguments #[(link) "\304\305\b\"\2031\306\307\b\"\310\306\311\b\"!\310\306\312\b\"!\313	\307\"\210\314
!\210\315\316\v\317 @_\320 \245!!+\207\304\321\b\"\203N\306\307\b\"\310\306\311\b\"!\313	\307\"\210\314
!*\207\313\b\307\"\207" [link path page height string-match "\\(.*\\)::\\([0-9]*\\)\\+\\+\\([[0-9]\\.*[0-9]*\\)" match-string 1 string-to-number 2 3 org-open-file pdf-view-goto-page image-set-window-vscroll round pdf-view-image-size frame-char-height "\\(.*\\)::\\([0-9]+\\)$"] 4 ("/home/wilk/.emacs.d/elpa/org-pdfview-20160125.1254/org-pdfview.elc" . 662)] 2)
  org-pdfview-open("/home/wilk/enseignement/2015-2016/topos/topo-tipe-beamer.pdf" "/home/wilk/enseignement/2015-2016/topos/topo-tipe-beamer.pdf")
--8<---------------cut here---------------end--------------->8---

Is it a bug in Org-mode or should I report the issue to the org-pdfview
author ?

Julien.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: org-pdfview-open doesn't work anymore
  2016-02-05  5:46 org-pdfview-open doesn't work anymore Julien Cubizolles
@ 2016-02-05  8:33 ` Michael Brand
  2016-02-05 13:36   ` Nicolas Goaziou
  2016-02-05 13:45   ` Michael Brand
  0 siblings, 2 replies; 12+ messages in thread
From: Michael Brand @ 2016-02-05  8:33 UTC (permalink / raw)
  To: Julien Cubizolles; +Cc: Org Mode

Hi Julien

On Fri, Feb 5, 2016 at 6:46 AM, Julien Cubizolles <j.cubizolles@free.fr> wrote:
> I've been using org-pdfview (from
> https://github.com/markus1189/org-pdfview) to have org-mode open pdf
> files generated during export.
>
> --8<---------------cut here---------------start------------->8---
> (pdf-tools-install)
> (eval-after-load 'org '(progn (require 'org-pdfview)
>                               (add-to-list 'org-file-apps '("\\.pdf\\'" . org-pdfview-open))
>                               ))
> --8<---------------cut here---------------end--------------->8---
>
> Since a recent upgrade, this fails with:
>
> --8<---------------cut here---------------start------------->8---
> (wrong-number-of-arguments #[(link) "\304\305 \"\2031\306\307 \"\310\306\311 \"!\310\306\312 \"!\313    \307\"\210\314
> !\210\315\316 \317 @_\320 \245!!+\207\304\321 \"\203N\306\307 \"\310\306\311 \"!\313    \307\"\210\314
> !*\207\313 \307\"\207" [link path page height string-match "\\(.*\\)::\\([0-9]*\\)\\+\\+\\([[0-9]\\.*[0-9]*\\)" match-string 1 string-to-number 2 3 org-open-file pdf-view-goto-page image-set-window-vscroll round pdf-view-image-size frame-char-height "\\(.*\\)::\\([0-9]+\\)$"] 4 ("/home/wilk/.emacs.d/elpa/org-pdfview-20160125.1254/org-pdfview.elc" . 662)] 2)
>   org-pdfview-open("/home/wilk/enseignement/2015-2016/topos/topo-tipe-beamer.pdf" "/home/wilk/enseignement/2015-2016/topos/topo-tipe-beamer.pdf")
> --8<---------------cut here---------------end--------------->8---
>
> Is it a bug in Org-mode or should I report the issue to the org-pdfview
> author ?

Due to lexical binding in org.el there was a change in
`org-file-apps', see Org News for version 9.0 and e. g. this thread:
http://thread.gmane.org/gmane.emacs.orgmode/104272
I think the most convenient would be if `org-open-file' tries to find
out that `cmd' in this case is a function with only one argument and
call it with just `file'.

@Nicolas: Is this reasonable for you to implement?

Michael

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: org-pdfview-open doesn't work anymore
  2016-02-05  8:33 ` Michael Brand
@ 2016-02-05 13:36   ` Nicolas Goaziou
  2016-02-10 12:55     ` Julien Cubizolles
  2016-02-05 13:45   ` Michael Brand
  1 sibling, 1 reply; 12+ messages in thread
From: Nicolas Goaziou @ 2016-02-05 13:36 UTC (permalink / raw)
  To: Michael Brand; +Cc: Org Mode, Julien Cubizolles

Hello,

Michael Brand <michael.ch.brand@gmail.com> writes:

> Hi Julien
>
> On Fri, Feb 5, 2016 at 6:46 AM, Julien Cubizolles <j.cubizolles@free.fr> wrote:
>> I've been using org-pdfview (from
>> https://github.com/markus1189/org-pdfview) to have org-mode open pdf
>> files generated during export.
>>
>> --8<---------------cut here---------------start------------->8---
>> (pdf-tools-install)
>> (eval-after-load 'org '(progn (require 'org-pdfview)
>>                               (add-to-list 'org-file-apps '("\\.pdf\\'" . org-pdfview-open))
>>                               ))
>> --8<---------------cut here---------------end--------------->8---
>>
>> Since a recent upgrade, this fails with:
>>
>> --8<---------------cut here---------------start------------->8---
>> (wrong-number-of-arguments #[(link) "\304\305 \"\2031\306\307 \"\310\306\311 \"!\310\306\312 \"!\313    \307\"\210\314
>> !\210\315\316 \317 @_\320 \245!!+\207\304\321 \"\203N\306\307 \"\310\306\311 \"!\313    \307\"\210\314
>> !*\207\313 \307\"\207" [link path page height string-match "\\(.*\\)::\\([0-9]*\\)\\+\\+\\([[0-9]\\.*[0-9]*\\)" match-string 1 string-to-number 2 3 org-open-file pdf-view-goto-page image-set-window-vscroll round pdf-view-image-size frame-char-height "\\(.*\\)::\\([0-9]+\\)$"] 4 ("/home/wilk/.emacs.d/elpa/org-pdfview-20160125.1254/org-pdfview.elc" . 662)] 2)
>>   org-pdfview-open("/home/wilk/enseignement/2015-2016/topos/topo-tipe-beamer.pdf" "/home/wilk/enseignement/2015-2016/topos/topo-tipe-beamer.pdf")
>> --8<---------------cut here---------------end--------------->8---
>>
>> Is it a bug in Org-mode or should I report the issue to the org-pdfview
>> author ?
>
> Due to lexical binding in org.el there was a change in
> `org-file-apps', see Org News for version 9.0 and e. g. this thread:
> http://thread.gmane.org/gmane.emacs.orgmode/104272
> I think the most convenient would be if `org-open-file' tries to find
> out that `cmd' in this case is a function with only one argument and
> call it with just `file'.
>
> @Nicolas: Is this reasonable for you to implement?

I think the simplest solution may be to follow the advice in ORG-NEWS
and use

  (lambda (file link) (org-pdfview-open file))


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: org-pdfview-open doesn't work anymore
  2016-02-05  8:33 ` Michael Brand
  2016-02-05 13:36   ` Nicolas Goaziou
@ 2016-02-05 13:45   ` Michael Brand
  2016-02-05 17:22     ` Nicolas Goaziou
  1 sibling, 1 reply; 12+ messages in thread
From: Michael Brand @ 2016-02-05 13:45 UTC (permalink / raw)
  To: Org Mode; +Cc: Julien Cubizolles

[-- Attachment #1: Type: text/plain, Size: 797 bytes --]

Hi Nicolas

On Fri, Feb 5, 2016 at 9:33 AM, Michael Brand
<michael.ch.brand@gmail.com> wrote:

> Due to lexical binding in org.el there was a change in
> `org-file-apps', see Org News for version 9.0 and e. g. this thread:
> http://thread.gmane.org/gmane.emacs.orgmode/104272
> I think the most convenient would be if `org-open-file' tries to find
> out that `cmd' in this case is a function with only one argument and
> call it with just `file'.

Only after a closer look I saw that the single parameter of
`org-pdfview-open' is not `file' but `link'. It is probably better for
`org-open-file' to not guess in case of `cmd' with a single parameter
whether it is meant to be `file' or `link'. That leads me to suggest
the attached patch to be reviewed that checks the function signature.

Michael

[-- Attachment #2: 0001-org-file-apps-add-migration-hint-for-function-signat.patch --]
[-- Type: text/x-patch, Size: 1797 bytes --]

From 9788cb03d2714cde555fbe2abb55ddd383a885c1 Mon Sep 17 00:00:00 2001
From: Michael Brand <michael.ch.brand@gmail.com>
Date: Fri, 5 Feb 2016 14:44:26 +0100
Subject: [PATCH] `org-file-apps' add migration hint for function signature

* lisp/org.el (org-open-file): Add an error when the function
signature does not match.
---
 lisp/org.el | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/lisp/org.el b/lisp/org.el
index 5a6c74e..9ebabf8 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -11323,6 +11323,18 @@ If the file does not exist, an error is thrown."
 		  (when (derived-mode-p 'org-mode) (org-reveal)))
 	    (search (org-link-search search))))
      ((functionp cmd)
+      ;; FIXME: Remove this check when most default installations of
+      ;; Emacs have at least Org 9.0.
+      (let ((arglist (help-function-arglist cmd)))
+	(when (or (memq '&optional arglist)
+		  (memq '&rest arglist)
+		  (/= 2 (length arglist)))
+	  (user-error
+	   (format
+	    "%s%s%S"
+	    "Please see Org News for version 9.0 about `org-file-apps', "
+	    "this function signature is wrong: "
+	    cmd))))
       (save-match-data
 	(set-match-data link-match-data)
 	(funcall cmd file link)))
@@ -11333,7 +11345,10 @@ If the file does not exist, an error is thrown."
       ;; `org-link-frame-setup' for an old usage of `org-file-apps'
       ;; with sexp instead of a function for `cmd'.
       (user-error
-       "Please see Org News for version 9.0 about `org-file-apps'"))
+       (format "%s%s%S"
+	       "Please see Org News for version 9.0 about `org-file-apps', "
+	       "this usage is wrong: "
+	       cmd)))
      (t (funcall (cdr (assq 'file org-link-frame-setup)) file)))
     (and (derived-mode-p 'org-mode)
 	 (eq old-mode 'org-mode)
-- 
2.1.3.dirty


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: org-pdfview-open doesn't work anymore
  2016-02-05 13:45   ` Michael Brand
@ 2016-02-05 17:22     ` Nicolas Goaziou
  2016-02-05 18:47       ` Michael Brand
  0 siblings, 1 reply; 12+ messages in thread
From: Nicolas Goaziou @ 2016-02-05 17:22 UTC (permalink / raw)
  To: Michael Brand; +Cc: Org Mode, Julien Cubizolles

Hello,

Michael Brand <michael.ch.brand@gmail.com> writes:

> +      ;; FIXME: Remove this check when most default installations of
> +      ;; Emacs have at least Org 9.0.
> +      (let ((arglist (help-function-arglist cmd)))
> +	(when (or (memq '&optional arglist)
> +		  (memq '&rest arglist)
> +		  (/= 2 (length arglist)))
> +	  (user-error
> +	   (format
> +	    "%s%s%S"
> +	    "Please see Org News for version 9.0 about `org-file-apps', "
> +	    "this function signature is wrong: "
> +	    cmd))))

I have the feeling there is some over-engineering involved there. 

In any case, instead of relying on `help-function-arglist', I suggest to
use something lightweight:

(condition-case err
    (funcall ...)
  (wrong-number-of-arguments
   (user-error "Please ..."))
  (invalid-function
   (user-error "Please ...")))


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: org-pdfview-open doesn't work anymore
  2016-02-05 17:22     ` Nicolas Goaziou
@ 2016-02-05 18:47       ` Michael Brand
  2016-02-05 22:43         ` Nicolas Goaziou
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Brand @ 2016-02-05 18:47 UTC (permalink / raw)
  To: Org Mode; +Cc: Julien Cubizolles

[-- Attachment #1: Type: text/plain, Size: 1658 bytes --]

Hi Nicolas

On Fri, Feb 5, 2016 at 6:22 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:

> Michael Brand <michael.ch.brand@gmail.com> writes:

>> +      ;; FIXME: Remove this check when most default installations of
>> +      ;; Emacs have at least Org 9.0.
>> +      (let ((arglist (help-function-arglist cmd)))
>> +     (when (or (memq '&optional arglist)
>> +               (memq '&rest arglist)
>> +               (/= 2 (length arglist)))
>> +       (user-error
>> +        (format
>> +         "%s%s%S"
>> +         "Please see Org News for version 9.0 about `org-file-apps', "
>> +         "this function signature is wrong: "
>> +         cmd))))
>
> I have the feeling there is some over-engineering involved there.

Also it should have allowed at least optional arguments

    (when (or (memq
               ;; Too complicated to parse regarding that such functions
               ;; are probably not useful here.
               '&rest
               arglist)
              (/= 2 (length (delete '&optional arglist))))
      (user-error

for e. g. a simple

    (add-to-list 'org-file-apps
                 (cons (concat org-player-file-extensions-regexp "$")
                       'org-player-play-file))

which refers to the existing

    (defun org-player-play-file (filename &optional pos)

> In any case, instead of relying on `help-function-arglist', I suggest to
> use something lightweight:
>
> (condition-case err
>     (funcall ...)
>   (wrong-number-of-arguments
>    (user-error "Please ..."))
>   (invalid-function
>    (user-error "Please ...")))

Of course. I didn't know about this possibility, remixed patch attached.

Michael

[-- Attachment #2: 0001-org-file-apps-add-migration-hint-for-function-signat.patch --]
[-- Type: text/x-patch, Size: 1914 bytes --]

From f1c382cabe5b34f52db22df70d5c25e02de2a18a Mon Sep 17 00:00:00 2001
From: Michael Brand <michael.ch.brand@gmail.com>
Date: Fri, 5 Feb 2016 19:42:55 +0100
Subject: [PATCH] `org-file-apps' add migration hint for function signature

* lisp/org.el (org-open-file): Add an error when the function
signature does not match.
---
 lisp/org.el | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index d4fb8a6..f6c5f89 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -11323,9 +11323,18 @@ If the file does not exist, an error is thrown."
 		  (when (derived-mode-p 'org-mode) (org-reveal)))
 	    (search (org-link-search search))))
      ((functionp cmd)
-      (save-match-data
-	(set-match-data link-match-data)
-	(funcall cmd file link)))
+      (condition-case err
+	  (save-match-data
+	    (set-match-data link-match-data)
+	    (funcall cmd file link))
+	;; FIXME: Remove this check when most default installations of
+	;; Emacs have at least Org 9.0.
+	((wrong-number-of-arguments invalid-function)
+	 (user-error
+	  (concat
+	   "Please see Org News for version 9.0 about `org-file-apps', "
+	   "error: "
+	   (prin1-to-string err))))))
      ((consp cmd)
       ;; FIXME: Remove this check when most default installations of
       ;; Emacs have at least Org 9.0.
@@ -11333,7 +11342,9 @@ If the file does not exist, an error is thrown."
       ;; `org-link-frame-setup' for an old usage of `org-file-apps'
       ;; with sexp instead of a function for `cmd'.
       (user-error
-       "Please see Org News for version 9.0 about `org-file-apps'"))
+       (concat "Please see Org News for version 9.0 about `org-file-apps', "
+	       "error: deprecated usage of "
+	       (prin1-to-string cmd))))
      (t (funcall (cdr (assq 'file org-link-frame-setup)) file)))
     (and (derived-mode-p 'org-mode)
 	 (eq old-mode 'org-mode)
-- 
2.1.3.dirty


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: org-pdfview-open doesn't work anymore
  2016-02-05 18:47       ` Michael Brand
@ 2016-02-05 22:43         ` Nicolas Goaziou
  2016-02-06  8:08           ` Michael Brand
  0 siblings, 1 reply; 12+ messages in thread
From: Nicolas Goaziou @ 2016-02-05 22:43 UTC (permalink / raw)
  To: Michael Brand; +Cc: Org Mode, Julien Cubizolles

Hello,

Michael Brand <michael.ch.brand@gmail.com> writes:

> +	;; FIXME: Remove this check when most default installations of
> +	;; Emacs have at least Org 9.0.
> +	((wrong-number-of-arguments invalid-function)
> +	 (user-error
> +	  (concat
> +	   "Please see Org News for version 9.0 about `org-file-apps', "
> +	   "error: "
> +	   (prin1-to-string err))))))

(user-error
 "Please see Org News for version 9.0 about `org-file-apps', error: %S"
 (nth 1 err))

You can use continuation character (i.e., "\" followed by a newline) if
string is too large.

>       ((consp cmd)
>        ;; FIXME: Remove this check when most default installations of
>        ;; Emacs have at least Org 9.0.
> @@ -11333,7 +11342,9 @@ If the file does not exist, an error is thrown."
>        ;; `org-link-frame-setup' for an old usage of `org-file-apps'
>        ;; with sexp instead of a function for `cmd'.
>        (user-error
> -       "Please see Org News for version 9.0 about `org-file-apps'"))
> +       (concat "Please see Org News for version 9.0 about `org-file-apps', "
> +	       "error: deprecated usage of "
> +	       (prin1-to-string cmd))))

Ditto.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: org-pdfview-open doesn't work anymore
  2016-02-05 22:43         ` Nicolas Goaziou
@ 2016-02-06  8:08           ` Michael Brand
  2016-02-06 16:41             ` Nicolas Goaziou
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Brand @ 2016-02-06  8:08 UTC (permalink / raw)
  To: Org Mode; +Cc: Julien Cubizolles

[-- Attachment #1: Type: text/plain, Size: 2112 bytes --]

Hi Nicolas

On Fri, Feb 5, 2016 at 11:43 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:

> Michael Brand <michael.ch.brand@gmail.com> writes:

>> +     ;; FIXME: Remove this check when most default installations of
>> +     ;; Emacs have at least Org 9.0.
>> +     ((wrong-number-of-arguments invalid-function)
>> +      (user-error
>> +       (concat
>> +        "Please see Org News for version 9.0 about `org-file-apps', "
>> +        "error: "
>> +        (prin1-to-string err))))))
>
> (user-error
>  "Please see Org News for version 9.0 about `org-file-apps', error: %S"
>  (nth 1 err))

The above does not provide (nth 0 err) which is the important error
type and (nth 2 err) which I find also helpful. See following zerop
examples.

What am I missing that it should not be

    ((wrong-number-of-arguments wrong-type-argument invalid-function)
     (user-error "Please see Org News for version 9.0 about \
`org-file-apps'--Lisp error: %S" err))

to get

    Please see Org News for version 9.0 about \
    `org-file-apps'--Lisp error: (wrong-number-of-arguments zerop 2)

    Please see Org News for version 9.0 about \
    `org-file-apps'--Lisp error: (wrong-type-argument numberp nil)

from

    (condition-case err (zerop nil nil) ...)

    (condition-case err (zerop nil) ...)

to mimic

    Debugger entered--Lisp error: (wrong-number-of-arguments zerop 2)

    Debugger entered--Lisp error: (wrong-type-argument numberp nil)

from

    (zerop nil nil)

    (zerop nil)

as far as possible?

I just notice that in our case at least in case of wrong-type-argument the
`cmd' is missing, so I suggest

    ((wrong-number-of-arguments wrong-type-argument invalid-function)
     (user-error "Please see Org News for version 9.0 about \
`org-file-apps'--Lisp error: The function %S leads to %S" cmd err))

for the attached intermediate patch version. For the above example of
(zerop nil) it would not only report "wrong-type-argument" and
"numberp" together with "nil" but also "zerop" which in our case is
the registered application, the source of the problem where the user
needs to look.

Michael

[-- Attachment #2: 0001-org-file-apps-add-migration-hint-for-function-signat.patch --]
[-- Type: text/x-patch, Size: 1727 bytes --]

From 873e99c9ee03594e45dd3e82d880c4b8a90d2192 Mon Sep 17 00:00:00 2001
From: Michael Brand <michael.brand@alumni.ethz.ch>
Date: Sat, 6 Feb 2016 09:03:17 +0100
Subject: [PATCH] `org-file-apps' add migration hint for function signature

* lisp/org.el (org-open-file): Add an error for when the function
signature does not match.
---
 lisp/org.el | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index cce4f3a..cacae0f 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -11325,15 +11325,21 @@ If the file does not exist, an error is thrown."
      ((functionp cmd)
       (save-match-data
 	(set-match-data link-match-data)
-	(funcall cmd file link)))
+	(condition-case err
+	    (funcall cmd file link)
+	  ;; FIXME: Remove this check when most default installations
+	  ;; of Emacs have at least Org 9.0.
+	  ((wrong-number-of-arguments wrong-type-argument invalid-function)
+	   (user-error "Please see Org News for version 9.0 about \
+`org-file-apps'--Lisp error: The function %S leads to %S" cmd err)))))
      ((consp cmd)
       ;; FIXME: Remove this check when most default installations of
       ;; Emacs have at least Org 9.0.
       ;; Heads-up instead of silently fall back to
       ;; `org-link-frame-setup' for an old usage of `org-file-apps'
       ;; with sexp instead of a function for `cmd'.
-      (user-error
-       "Please see Org News for version 9.0 about `org-file-apps'"))
+      (user-error "Please see Org News for version 9.0 about \
+`org-file-apps'--error: Deprecated usage of %S" cmd))
      (t (funcall (cdr (assq 'file org-link-frame-setup)) file)))
     (and (derived-mode-p 'org-mode)
 	 (eq old-mode 'org-mode)
-- 
2.4.9 (Apple Git-60)


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: org-pdfview-open doesn't work anymore
  2016-02-06  8:08           ` Michael Brand
@ 2016-02-06 16:41             ` Nicolas Goaziou
  2016-02-07 10:13               ` Michael Brand
  0 siblings, 1 reply; 12+ messages in thread
From: Nicolas Goaziou @ 2016-02-06 16:41 UTC (permalink / raw)
  To: Michael Brand; +Cc: Org Mode, Julien Cubizolles

Hello,

Michael Brand <michael.ch.brand@gmail.com> writes:

> I just notice that in our case at least in case of wrong-type-argument the
> `cmd' is missing, so I suggest
>
>     ((wrong-number-of-arguments wrong-type-argument invalid-function)
>      (user-error "Please see Org News for version 9.0 about \
> `org-file-apps'--Lisp error: The function %S leads to %S" cmd err))
>
> for the attached intermediate patch version. For the above example of
> (zerop nil) it would not only report "wrong-type-argument" and
> "numberp" together with "nil" but also "zerop" which in our case is
> the registered application, the source of the problem where the user
> needs to look.

IMO, notifying user that there's something rotten in the state of
`org-file-apps' is enough. There's no need to go into the gory details
of the problem.

Anyway, if you feel strongly about it, your patch looks good and you can
push it.

Thank you.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: org-pdfview-open doesn't work anymore
  2016-02-06 16:41             ` Nicolas Goaziou
@ 2016-02-07 10:13               ` Michael Brand
  2016-02-07 11:06                 ` Nicolas Goaziou
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Brand @ 2016-02-07 10:13 UTC (permalink / raw)
  To: Org Mode; +Cc: Julien Cubizolles

[-- Attachment #1: Type: text/plain, Size: 569 bytes --]

Hi Nicolas

On Sat, Feb 6, 2016 at 5:41 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:

> IMO, notifying user that there's something rotten in the state of
> `org-file-apps' is enough. There's no need to go into the gory details
> of the problem.

I agree and finally found what I was missing: Add `debug' to the
handler of `condition-case' to finally not disable support of further
investigation with `toggle-debug-on-error'. It obsoletes my trials to
provide enough context about the Lisp error in the handler itself.
Remixed patch for review attached.

Michael

[-- Attachment #2: 0001-org-file-apps-add-migration-hint-for-function-signat.patch --]
[-- Type: text/x-patch, Size: 1716 bytes --]

From 96aa89840c15c71c534faa0ce265530d5ff88c0a Mon Sep 17 00:00:00 2001
From: Michael Brand <michael.brand@alumni.ethz.ch>
Date: Sun, 7 Feb 2016 11:07:56 +0100
Subject: [PATCH] `org-file-apps' add migration hint for function signature

* lisp/org.el (org-open-file): Add a user error for when the function
signature does not match.
---
 lisp/org.el | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index cce4f3a..e77fd4a 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -11325,15 +11325,22 @@ If the file does not exist, an error is thrown."
      ((functionp cmd)
       (save-match-data
 	(set-match-data link-match-data)
-	(funcall cmd file link)))
+	(condition-case nil
+	    (funcall cmd file link)
+	  ;; FIXME: Remove this check when most default installations
+	  ;; of Emacs have at least Org 9.0.
+	  ((debug wrong-number-of-arguments wrong-type-argument
+	    invalid-function)
+	   (user-error "Please see Org News for version 9.0 about \
+`org-file-apps'--Lisp error: %S" cmd)))))
      ((consp cmd)
       ;; FIXME: Remove this check when most default installations of
       ;; Emacs have at least Org 9.0.
       ;; Heads-up instead of silently fall back to
       ;; `org-link-frame-setup' for an old usage of `org-file-apps'
       ;; with sexp instead of a function for `cmd'.
-      (user-error
-       "Please see Org News for version 9.0 about `org-file-apps'"))
+      (user-error "Please see Org News for version 9.0 about \
+`org-file-apps'--Error: Deprecated usage of %S" cmd))
      (t (funcall (cdr (assq 'file org-link-frame-setup)) file)))
     (and (derived-mode-p 'org-mode)
 	 (eq old-mode 'org-mode)
-- 
2.4.9 (Apple Git-60)


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: org-pdfview-open doesn't work anymore
  2016-02-07 10:13               ` Michael Brand
@ 2016-02-07 11:06                 ` Nicolas Goaziou
  0 siblings, 0 replies; 12+ messages in thread
From: Nicolas Goaziou @ 2016-02-07 11:06 UTC (permalink / raw)
  To: Michael Brand; +Cc: Org Mode, Julien Cubizolles

Hello,

Michael Brand <michael.ch.brand@gmail.com> writes:

> Remixed patch for review attached.

It looks good. Please apply it on master.

Thank you.

Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: org-pdfview-open doesn't work anymore
  2016-02-05 13:36   ` Nicolas Goaziou
@ 2016-02-10 12:55     ` Julien Cubizolles
  0 siblings, 0 replies; 12+ messages in thread
From: Julien Cubizolles @ 2016-02-10 12:55 UTC (permalink / raw)
  To: emacs-orgmode

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:


> I think the simplest solution may be to follow the advice in ORG-NEWS
> and use
>
>   (lambda (file link) (org-pdfview-open file))

It's working, thanks.

Julien.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-02-10 12:55 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-05  5:46 org-pdfview-open doesn't work anymore Julien Cubizolles
2016-02-05  8:33 ` Michael Brand
2016-02-05 13:36   ` Nicolas Goaziou
2016-02-10 12:55     ` Julien Cubizolles
2016-02-05 13:45   ` Michael Brand
2016-02-05 17:22     ` Nicolas Goaziou
2016-02-05 18:47       ` Michael Brand
2016-02-05 22:43         ` Nicolas Goaziou
2016-02-06  8:08           ` Michael Brand
2016-02-06 16:41             ` Nicolas Goaziou
2016-02-07 10:13               ` Michael Brand
2016-02-07 11:06                 ` Nicolas Goaziou

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).