Skip to content
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

Minor touch-ups #1519

Merged
merged 2 commits into from Apr 3, 2018
Merged

Minor touch-ups #1519

merged 2 commits into from Apr 3, 2018

Conversation

basil-conto
Copy link
Collaborator

@basil-conto basil-conto commented Apr 3, 2018

Changelog

  • colir.el: Require cl-lib dependency.
  • ivy.el (ivy-quit-and-run):
    • Add indentation spec.
    • Eagerly demote our own errors for clearer error messages.
    • Call abort-recursive-edit directly, irrespective of delete-selection-mode.
  • (ivy-alt-done):
    swiper.el (swiper-avy): Reindent its call sites.

Rationale

Demoting errors in the ivy-quit-and-run timer function means errors are reported along the lines of Error: (error "foo") rather than Error running timer: (error "foo"), the latter of which leaks the macro's abstraction.

Discussion

Further to #606 (comment), I have been trying to refactor ivy-quit-and-run to avoid:

  1. changing the error-message of quit; and
  2. using timers,

but I have not been successful.

For a second I thought (1) could be solved by temporarily disabling command-error-function, as described in (elisp) Processing of Errors, but this variable does not take effect for local quits. I'm not sure whether increasing its scope would constitute a worthwhile feature request.

What to me seems to be the canonical alternative to (2) is using post-command-hook, e.g.

(defmacro ivy-quit-and-run (&rest body)
  "Quit the minibuffer and run BODY afterwards."
  (declare (indent 0))
  (let ((nonce (make-symbol "nonce")))
    `(letrec ((,nonce (lambda ()
                        (remove-hook 'post-command-hook ,nonce)
                        (with-local-quit
                          (with-demoted-errors "Error: %S"
                            ,@body)))))
       (add-hook 'post-command-hook ,nonce)
       (abort-recursive-edit))))

The downside of which is the requirement of lexical-binding at the call site. I think the reason minibuffer-with-setup-hook works in a similar way, irrespective of lexical-binding, is because it runs entirely within the same dynamic scope/extent.

Perhaps another alternative would be to run BODY in a recursive edit, but I'm both unsure this is even possible and far from convinced it is any better than using timers.

basil-conto added 2 commits Apr 3, 2018
ivy.el (ivy-quit-and-run): Add indentation spec.  Eagerly demote our
own errors for clearer error messages.  Call abort-recursive-edit
directly, irrespective of delete-selection-mode.
(ivy-alt-done):
swiper.el (swiper-avy): Reindent its call sites.
@abo-abo abo-abo merged commit b11deda into abo-abo:master Apr 3, 2018
1 of 2 checks passed
@abo-abo
Copy link
Owner

@abo-abo abo-abo commented Apr 3, 2018

Thanks.

What to me seems to be the canonical alternative to (2) is using post-command-hook, e.g.

I'm not yet set on whether I like this more than the old thing. My immediate reaction is that it's a bit more complex; I'll have to think about this more.

@basil-conto basil-conto deleted the blc/misc branch Apr 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants