emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* Quit and Error in org-export--dispatch-action
@ 2019-12-05  7:42 Takaaki Ishikawa
  2019-12-05 10:27 ` Kyle Meyer
  0 siblings, 1 reply; 7+ messages in thread
From: Takaaki Ishikawa @ 2019-12-05  7:42 UTC (permalink / raw)
  To: orgmode list

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

Dear Nicolas and all,

The org-export provides a quitting option for user by typing `q`.
This is nice feature but it is implemented with an error function.
For me, it is not actually an error, it is one of the user actions,
and when `debug-on-error` is `t`, the Backtrace buffer will be
popped up every time. It is annoying.

Please find a patch to replace error function with a simple message.
What do you think?

Best regards,
Takaaki

--
Takaaki ISHIKAWA <takaxp@ieee.org>

[-- Attachment #2: ox.patch --]
[-- Type: application/octet-stream, Size: 701 bytes --]

diff --git a/lisp/ox.el b/lisp/ox.el
index 5b4134ecc..98846540d 100644
--- a/lisp/ox.el
+++ b/lisp/ox.el
@@ -6929,8 +6929,8 @@ options as CDR."
       (org-export--dispatch-ui options first-key expertp))
      ;; q key at first level aborts export.  At second level, cancel
      ;; first key instead.
-     ((eq key ?q) (if (not first-key) (error "Export aborted")
-		    (org-export--dispatch-ui options nil expertp)))
+     ((eq key ?q) (if first-key (org-export--dispatch-ui options nil expertp)
+		    (message "Export aborted") '(ignore)))
      ;; Help key: Switch back to standard interface if expert UI was
      ;; active.
      ((eq key ??) (org-export--dispatch-ui options first-key nil))

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

* Re: Quit and Error in org-export--dispatch-action
  2019-12-05  7:42 Quit and Error in org-export--dispatch-action Takaaki Ishikawa
@ 2019-12-05 10:27 ` Kyle Meyer
  2019-12-05 13:59   ` Takaaki Ishikawa
  0 siblings, 1 reply; 7+ messages in thread
From: Kyle Meyer @ 2019-12-05 10:27 UTC (permalink / raw)
  To: Takaaki Ishikawa, orgmode list

Hi Takaaki,

Takaaki Ishikawa <takaxp@ieee.org> writes:

> The org-export provides a quitting option for user by typing `q`.
> This is nice feature but it is implemented with an error function.
> For me, it is not actually an error, it is one of the user actions,
> and when `debug-on-error` is `t`, the Backtrace buffer will be
> popped up every time. It is annoying.

True, that shouldn't be treated as a plain error.

> Please find a patch to replace error function with a simple message.
> What do you think?
>
>[...]
>
> --- a/lisp/ox.el
> +++ b/lisp/ox.el
> @@ -6929,8 +6929,8 @@ options as CDR."
>        (org-export--dispatch-ui options first-key expertp))
>       ;; q key at first level aborts export.  At second level, cancel
>       ;; first key instead.
> -     ((eq key ?q) (if (not first-key) (error "Export aborted")
> -		    (org-export--dispatch-ui options nil expertp)))
> +     ((eq key ?q) (if first-key (org-export--dispatch-ui options nil expertp)
> +		    (message "Export aborted") '(ignore)))

Hmm, what about instead replacing the call to `error' with a call to
`user-error'?  If that works for you, could you send an updated patch
with a commit message?  (Org's commit message conventions are described
at <https://orgmode.org/worg/org-contribute.html#commit-messages>.)

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

* Re: Quit and Error in org-export--dispatch-action
  2019-12-05 10:27 ` Kyle Meyer
@ 2019-12-05 13:59   ` Takaaki Ishikawa
  2019-12-06  3:48     ` Kyle Meyer
  0 siblings, 1 reply; 7+ messages in thread
From: Takaaki Ishikawa @ 2019-12-05 13:59 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: orgmode list

Dear Kyle and all,

Using user-error is another way, but it does not work for me
because user-error stops the org-export-dispatch.
I would like to keep the session to do an action after
the completing org-export-dispatch something like this:

  (defun my-org-export-dispatch (f ARG)
    (interactive "P")
    (if (< (frame-width) 160)
        (apply f ARG)
      (split-window-right)
      (apply f ARG)
      (delete-window)))
  (advice-add 'org-export-dispatch :around #'my-org-export-dispatch)

So I still prefer to replace the error function with a simple message function.
If you agree with this idea, I'll produce an appropriate patch for this
as you kindly instructed.

Best,
Takaaki


--
Takaaki ISHIKAWA <takaxp@ieee.org>

2019年12月5日(木) 19:27 Kyle Meyer <kyle@kyleam.com>:
>
> Hi Takaaki,
>
> Takaaki Ishikawa <takaxp@ieee.org> writes:
>
> > The org-export provides a quitting option for user by typing `q`.
> > This is nice feature but it is implemented with an error function.
> > For me, it is not actually an error, it is one of the user actions,
> > and when `debug-on-error` is `t`, the Backtrace buffer will be
> > popped up every time. It is annoying.
>
> True, that shouldn't be treated as a plain error.
>
> > Please find a patch to replace error function with a simple message.
> > What do you think?
> >
> >[...]
> >
> > --- a/lisp/ox.el
> > +++ b/lisp/ox.el
> > @@ -6929,8 +6929,8 @@ options as CDR."
> >        (org-export--dispatch-ui options first-key expertp))
> >       ;; q key at first level aborts export.  At second level, cancel
> >       ;; first key instead.
> > -     ((eq key ?q) (if (not first-key) (error "Export aborted")
> > -                 (org-export--dispatch-ui options nil expertp)))
> > +     ((eq key ?q) (if first-key (org-export--dispatch-ui options nil expertp)
> > +                 (message "Export aborted") '(ignore)))
>
> Hmm, what about instead replacing the call to `error' with a call to
> `user-error'?  If that works for you, could you send an updated patch
> with a commit message?  (Org's commit message conventions are described
> at <https://orgmode.org/worg/org-contribute.html#commit-messages>.)

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

* Re: Quit and Error in org-export--dispatch-action
  2019-12-05 13:59   ` Takaaki Ishikawa
@ 2019-12-06  3:48     ` Kyle Meyer
  2019-12-09  4:58       ` Takaaki Ishikawa
  0 siblings, 1 reply; 7+ messages in thread
From: Kyle Meyer @ 2019-12-06  3:48 UTC (permalink / raw)
  To: Takaaki Ishikawa; +Cc: orgmode list

Takaaki Ishikawa <takaxp@ieee.org> writes:

> Dear Kyle and all,
>
> Using user-error is another way, but it does not work for me
> because user-error stops the org-export-dispatch.
> I would like to keep the session to do an action after
> the completing org-export-dispatch something like this:
>
>   (defun my-org-export-dispatch (f ARG)
>     (interactive "P")
>     (if (< (frame-width) 160)
>         (apply f ARG)
>       (split-window-right)
>       (apply f ARG)
>       (delete-window)))
>   (advice-add 'org-export-dispatch :around #'my-org-export-dispatch)
>
> So I still prefer to replace the error function with a simple message function.

Thanks for providing more context.  So if I'm understanding correctly,
the point here is that for your use case/setup you'd like to call
delete-window even when you select 'q' within the org-export-dispatch
call.  Signaling a user-error doesn't make this much more difficult: you
can wrap the `(apply f ...)' call within a condition-case.

I see two related advantages of sticking to signaling an error here:

 * It stays close to what the current code does.  Continuing to signal
   an error but replacing `error' with `user-error' reduces the risk of
   bugs or unintended changes in behavior while avoiding showing a
   backtrace when the caller aborts and debug-on-error is true (the
   initial issue reported in this thread).

 * 'q' is described as aborting, so I think it's confusing to make 'q'
   instead execute a no-op function and continue with the remaining code
   in the outer functions, which at the moment boils down to code in
   org-export-dispatch.  For example, with your suggested change,
   issuing 'q' will result in `ignore' being saved as the last export
   action, and it's hard to imagine that's an action the user would
   expect or want to repeat when calling org-export-dispatch with a
   prefix argument.

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

* Re: Quit and Error in org-export--dispatch-action
  2019-12-06  3:48     ` Kyle Meyer
@ 2019-12-09  4:58       ` Takaaki Ishikawa
  2019-12-09 10:39         ` Kyle Meyer
  0 siblings, 1 reply; 7+ messages in thread
From: Takaaki Ishikawa @ 2019-12-09  4:58 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: orgmode list

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

Dear Kyle and all,

Thank you for your kind feedback.

> Thanks for providing more context.  So if I'm understanding correctly,
> the point here is that for your use case/setup you'd like to call
> delete-window even when you select 'q' within the org-export-dispatch
> call. Signaling a user-error doesn't make this much more difficult: you
> can wrap the `(apply f ...)' call within a condition-case.

Yes. But I found out `C-g' also exits `org-export-dispatch’ and
it is difficult for me to catch `C-g' signal in the original advice function.

So I’ve tried again and produced the following code to support calling any 
additional functions before and after `org-export-dispatch’ even if user types `q` or `C-g`.
In this case, using user-error is OK for me :)

#+begin_src emacs-lisp
(with-eval-after-load "ox"
  (defvar my-org-export-before-hook nil)
  (add-hook 'my-org-export-before-hook #'split-window-horizontally)

  (defvar my-org-export-after-hook nil)
  (add-hook 'my-org-export-after-hook #'delete-window)

  (defun my-org-export--post-processing ()
    (when (eq this-command 'org-export-dispatch)
      (run-hooks 'my-org-export-after-hook))
    (remove-hook 'post-command-hook #'my-org-export--post-processing))

  (defun my-org-export-dispatch (f ARG)
    (cond (org-export-dispatch-use-expert-ui
           (apply f ARG))
          ((> (frame-width) 160)
           (when my-org-export-after-hook
             (add-hook 'post-command-hook #'my-org-export--post-processing))
           (run-hooks 'my-org-export-before-hook)
           (apply f ARG))
          (t
           (apply f ARG))))
  (advice-add 'org-export-dispatch :around #'my-org-export-dispatch))
#+end_src

I created a patch for this issue. Please find an attached patch.
It is a really tiny patch. But I have already signed the copyright
assignment with FSF.


> I see two related advantages of sticking to signaling an error here:
> 
> * It stays close to what the current code does.  Continuing to signal
>   an error but replacing `error' with `user-error' reduces the risk of
>   bugs or unintended changes in behavior while avoiding showing a
>   backtrace when the caller aborts and debug-on-error is true (the
>   initial issue reported in this thread).
> 
> * 'q' is described as aborting, so I think it's confusing to make 'q'
>   instead execute a no-op function and continue with the remaining code
>   in the outer functions, which at the moment boils down to code in
>   org-export-dispatch.  For example, with your suggested change,
>   issuing 'q' will result in `ignore' being saved as the last export
>   action, and it's hard to imagine that's an action the user would
>   expect or want to repeat when calling org-export-dispatch with a
>   prefix argument.

I partially agree with you because the dispatcher shows it
as “[q] Exit”. This is actually different from “[q] Aborting”.
Typing `q` is completely intended by user to exit the procedure.
If something is happened accidentally in the procedure,
then, IMO, we should say it as “aborting” and update the code appropriately.

Anyway, replacing with `user-error` is OK.

Best,
Takaaki



[-- Attachment #2: 0001-ox.el-Replace-error-with-user-error-to-exit-org-expo.patch --]
[-- Type: application/octet-stream, Size: 1220 bytes --]

From b22ed4854e7776effa62f01af47691800a76a9ff Mon Sep 17 00:00:00 2001
From: Takaaki ISHIKAWA <takaxp@ieee.org>
Date: Mon, 9 Dec 2019 13:08:56 +0900
Subject: [PATCH] ox.el: Replace error with user-error to exit
 org-export-dispatch

* lisp/ox.el (org-export--dispatch-action): Replace error with user-error
(org-export--dispatch-action): Replace an error function with a user-error so that user can quit `org-export-dispatch' without entering debugging mode.

Modified from a patch proposal by Takaaki Ishikawa.
---
 lisp/ox.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/ox.el b/lisp/ox.el
index 5b4134ecc..10286e18f 100644
--- a/lisp/ox.el
+++ b/lisp/ox.el
@@ -6929,7 +6929,7 @@ options as CDR."
       (org-export--dispatch-ui options first-key expertp))
      ;; q key at first level aborts export.  At second level, cancel
      ;; first key instead.
-     ((eq key ?q) (if (not first-key) (error "Export aborted")
+     ((eq key ?q) (if (not first-key) (user-error "Export aborted")
 		    (org-export--dispatch-ui options nil expertp)))
      ;; Help key: Switch back to standard interface if expert UI was
      ;; active.
-- 
2.21.0 (Apple Git-122.2)


[-- Attachment #3: Type: text/plain, Size: 4 bytes --]






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

* Re: Quit and Error in org-export--dispatch-action
  2019-12-09  4:58       ` Takaaki Ishikawa
@ 2019-12-09 10:39         ` Kyle Meyer
  2019-12-09 15:02           ` Takaaki Ishikawa
  0 siblings, 1 reply; 7+ messages in thread
From: Kyle Meyer @ 2019-12-09 10:39 UTC (permalink / raw)
  To: Takaaki Ishikawa; +Cc: orgmode list

Takaaki Ishikawa <takaxp@ieee.org> writes:

[...]

> I created a patch for this issue. Please find an attached patch.

Thanks!  Applied in c7ad3f884 (with minor cosmetic tweaks to the commit
message).

> It is a really tiny patch. But I have already signed the copyright
> assignment with FSF.

Great, I've added you to the list at
https://orgmode.org/worg/org-contribute.html

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

* Re: Quit and Error in org-export--dispatch-action
  2019-12-09 10:39         ` Kyle Meyer
@ 2019-12-09 15:02           ` Takaaki Ishikawa
  0 siblings, 0 replies; 7+ messages in thread
From: Takaaki Ishikawa @ 2019-12-09 15:02 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: orgmode list

Dear Kyle,

Confirmed. Thanks!

Best,
Takaaki

2019年12月9日(月) 19:39 Kyle Meyer <kyle@kyleam.com>:

>
> Takaaki Ishikawa <takaxp@ieee.org> writes:
>
> [...]
>
> > I created a patch for this issue. Please find an attached patch.
>
> Thanks!  Applied in c7ad3f884 (with minor cosmetic tweaks to the commit
> message).
>
> > It is a really tiny patch. But I have already signed the copyright
> > assignment with FSF.
>
> Great, I've added you to the list at
> https://orgmode.org/worg/org-contribute.html

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

end of thread, other threads:[~2019-12-09 15:02 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-05  7:42 Quit and Error in org-export--dispatch-action Takaaki Ishikawa
2019-12-05 10:27 ` Kyle Meyer
2019-12-05 13:59   ` Takaaki Ishikawa
2019-12-06  3:48     ` Kyle Meyer
2019-12-09  4:58       ` Takaaki Ishikawa
2019-12-09 10:39         ` Kyle Meyer
2019-12-09 15:02           ` Takaaki Ishikawa

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