oantolin / embark

Emacs Mini-Buffer Actions Rooted in Keymaps
GNU General Public License v3.0
946 stars 56 forks source link

Announce the new features: Detailed Action Indicator, Highlighting and Cycling of the Target at Point #291

Closed minad closed 3 years ago

minad commented 3 years ago

gif

movie

oantolin commented 3 years ago

Yes, an announcement is desperately needed. Definitely some changes to the README, maybe also a reddit post?

oantolin commented 3 years ago

Also a new tag release (although I want to wait a couple of days for that in case of undiscovered bugs).

minad commented 3 years ago

Okay, do you mind if I make a post with the recent news? I think it would be good to have a post now while the recent developments are still new. And then after you tagged a new release another announcement post listing more changes in detail. I am kind of excited by the recent changes and I am eager to hear what people think.

minad commented 3 years ago

If you want to post it only once that's okay - some projects post a lot of updates, some only releases. I am just unreasonably excited by the new developments :sweat_smile:

oantolin commented 3 years ago

Go ahead! To post on reddit I don't think we need to wait.

oantolin commented 3 years ago

These three things together really add a whole new level of polish to Embark. It feels more professional now, like Consult. 😉

minad commented 3 years ago

Posted here: https://www.reddit.com/r/emacs/comments/osx5t9/recent_embark_developments_detailed_action/

Well Consult is professional, I mean already the name. It is supposed for enterprise Emacs installs or professional academic installs :smile:

minad commented 3 years ago

Hopefully we get some interesting feedback :)

oantolin commented 3 years ago

What is going on at the top now? I see two copies of the gif.

minad commented 3 years ago

I abused github as a gif filehoster :-P (I really don't understand how this reddit works)

jdtsmith commented 3 years ago

These are great! So glad my suggestion of highlighting the match made it in. This really adds to the functionality/discoverability. Now if we can pre-highlight forward/backward showing matches (like isearch does) I'll never need M-s . C-s again. Perhaps n / p can enable temporary highlighting of the symbol at point?

oantolin commented 3 years ago

I'm not sure how much special treatment the next/previous symbol actions should get...

In the meantime, if you want to highlight occurrences of the symbol at point there is a toggle highlight action bound to capital H.

jdtsmith commented 3 years ago

I saw H which looks useful, but it leaves the highlighting active and then exits, whereas I'd usually be after the opposite: temporarily highlight and then immediate nearby navigation with n/p. (I love that you can then convert this into a consult-line!). Now that embark has non-exiting actions available from outside the minibuffer (what the * means?), perhaps it makes sense to allow non-exiting actions with prefix. E.g. C-u H to turn on hi-lighting but not exit. That would get me part the way there.

oantolin commented 3 years ago

For now, you can add embark-toggle-highlight to embark-repeat-commands if you never want it to exit embark-act. If you want to have the choice of exiting or not, you can define an alias and make that one repeatable

(defalias 'embark-toggle-highlight-noexit 'embark-toggle-highlight)
(add-to-list 'embark-repeat-commands 'embark-toggle-highlight-noexit)
(define-key embark-identifier-map (kbd "M-h") 'embark-toggle-highlight-noexit)

I realize this is not as comfortable as having the highlighting automatic, since you need to turn it on and off.

As for having the prefix indicate non-exiting, as in your C-u H example, I think that's probably not a good idea. Currently the prefix argument means, pretty logically, to run the action with a prefix argument, and some actions definitely need the option of using their prefix argument. Maybe the thing to do would be to have a separate key function as a kind of prefix argument for this, maybe M-c for "continue", so M-c before any action would mark it as temporarily repeatable.

oantolin commented 3 years ago

Another thing you could do, that is more automatic, is to have the symbol target finder highlight all occurrences of the the symbol at point and unhighlight them in a post-command hook:

(defun temporarily-highlight-symbol-at-point (target)
  (when target
    (let ((regexp (find-tag-default-as-symbol-regexp))
          (hook (make-symbol "hook")))
      (fset hook (lambda ()
                   (remove-hook 'post-command-hook hook)
                   (unhighlight-regexp regexp)))
      (add-hook 'post-command-hook hook)
      (highlight-symbol-at-point))
    target))

(advice-add 'embark-target-identifier-at-point
            :filter-return 'temporarily-highlight-symbol-at-point)

This is pretty close to what you want for just symbol targets. As far as I can tell the only problem is that the highlighting sticks around even if you cycle to other targets. Oh, and that if you select embark-toggle-highlight as the action it turns the highlighting off, of course!

minad commented 3 years ago

We should not add special treatment for symbols here. This would go very much against the Zen of Embark as I see it ;)

But we could consider adding general hooks target-activated-hook? It could even be an alist with the type as key. In principle this is very much the same as the indicator. What about making embark-indicator an alist with type as key and hooks as value? Then we can add a symbol indicator which highlights all the symbols after a delay and it could reuse the hi-lock functionality as in your temporarily-highlight-symbol-at-point function.

minad commented 3 years ago

I really like this idea btw, right now embark--highlight-target is ad-hoc functionality, it should instead also be implemented as a general action indicator, such that people can turn off this annoying highlighting if they want ;)

EDIT: Since I like the generalization idea so much, I took the liberty to create issue https://github.com/oantolin/embark/issues/330.

jdtsmith commented 3 years ago

Currently the prefix argument means, pretty logically, to run the action with a prefix argument, and some actions definitely need the option of using their prefix argument.

I wondered about that. In fact I had a related issue regarding embark and prefixes. It doesn't seem prefixes to export get passed on to the exported command. For example I regularly use (with isearch/occur) M-s ., M-5 M-s o to get 5 lines of context in an occur buffer. When exporting from consult-line etc. to an occur buffer, I had hoped a M-5 C-, E would accomplish the same thing, but the prefix info doesn't seem to make it through.

Thanks both for your work on this (which I mostly missed while messing around with consult-imenu!).

jdtsmith commented 3 years ago

I'll open a separate issue. Update: #337