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
Some nits about the new popup #17
Comments
I see
Transients can be "suspended" using While a transient is active the Allowing arbitrary commands to change those things too would mean that a suffix command wouldn't be able to do that for the benefit of the next suffix because then we would be forced to restore the location before invoking a suffix. That's not very relevant right now because all existing transients are used to pass arguments to some suffix command (or as plain command dispatchers), but eventually I want to use transients for the sort of thing that hydras are used for, i.e. "sticky prefixes". By the way, Hydra has these limitations too.
Someone else mentioned this on Reddit. This is a use-case that had not occurred to me. Unfortunately using isearch in transient is not possible. It probably would be possible to implement a isearch-like feature, but it would not be as good, hard to implement, and probably a bit brittle. I'm afraid you will have to live without this. At least I don't intend to implement this. I am sorry to say that you probably won't get these things back. On the bright side, that covers all lost functionality I believe, so at least you are not in for any other surprises.
That's possible: (define-key transient-map "q" 'transient-quit-one)
(define-key transient-edit-map "q" 'transient-quit-one)
(define-key transient-sticky-map "q" 'transient-quit-seq) But that will shadow every transient-specific binding for I have chosen |
Hi, there. Is there any way to keep using magit-popup in the future? Because magit popup splits window inside, so that it won't change frame layout. |
Maybe doing something like isearch isn't as hard as I imagine initially. I'll probably take a stab at it eventually but it could be a while. Using isearch itself could be problematic because it also uses transient keymaps.
No.
I don't quite understand what that means. What worked with |
Is there somewhere some documentation about the transition? I used to call in my init file magit-define-popup-* functions, that is now all broken, there were no obsoletion warnings, things just broke with the last upgrade. I guess not all Magit users follow the development process... I now added a hard require to magit-popup, which I reinstalled, to my init file so I can use Emacs again. Are there replacements for the define-popup-* functions? |
I have added some dummy functions with instructions in 732de659a57914cf0624edfb261b470ad046a3e1. |
And in case a concrete example is useful: Here's the diff of my configuration's conversion from magit-popup to transient. I'm still new to the transient stuff, so it might not be the best way, but it works. |
Oh, the wiki page has not yet been done. Let's see if kyleams posted diff is sufficient. |
Hmm, seems it's not trivial. I'll wait until the wiki page has been filled. Thanks so far. |
@tarsius Ok, thanks for the wiki. Starting to migrate... (transient-append-suffix 'magit-stash "i" and try to add something after the new "j", I get an error not telling me much. Adding something else after "i" also doesn't work as expected: say I try to insert a "k" also after "i", then the "j" entry seems to be removed. |
Ok, the rest worked. BTW, you only have to have "vdiff-magit" installed to trigger the warning. I found the culprit by adding (debug) to the definition of 'magit--magit-popup-warning'. |
@tarsius thanks a lot. From my side the issue is resolved. Feel free to close it when you see fit. |
FWIW, display-warning logs all warnings in one buffer, one warning after the other, so point has to be at the end of the buffer. If you care, you could use your own popup buffer, but it's really not important. |
One more thing: The manual that is shipped with the Elpa package still uses obsolete names, e.g. "magit-run-popup" or "magit-dispatch-popup". These commands don't work anymore, so this should probably be fixed soon. |
For the usual split window configuration that means that we get a full width popup which is quite unnatural on large screens, as compared to a popup confined to the magit window. |
I like the simplicity of |
I have just pushed the That also restores the ability to scroll the popup window again. Isearch and selecting a suffix using the arrow keys or the mouse is still not possible again. Not sure if I want to bring that back. Please give it a try. |
See #18 for how to use |
Except for I intend to look into what it would entail to bring back |
I liked the old magit-popup behavior better. If I had a split frame with one window on the left, and a magit buffer on the right, pressing With transient overtaking the echo area (and a huge amount of vertical space) it pushes everything else scrunched up (both the window on the left side, which used to be unaffected, as well as the main magit buffer). Overall, a very jarring experience. Is there anyway to customize transient to be more horizontal and less vertical? It's only using about 1/10th of the horizontal space, and eating up 1/3rd of my vertical real estate. |
@MatthewFluet thank you so much! |
Having a similar issue here. I'd like the "old" magit popup behavior, and I've customized Previous behavior was: I'd run The worst part is when I hit ctrl-g to abort, it un-splits my window. magit 20190404, transient 20190319 |
I cannot reproduce this. My guess is that you have installed some other package that controls how buffers are being displayed. Try to reproduce the issue using |
OK, it works as intended with |
Since transient mode can be suspended, would it be possible to keep the bindings in the transient buffer and allow the default to be set to suspended vs activated? |
As always there are trade-offs. The isearch issue is a problem ot Showing documentation is essentially the same in The modal nature of the transients is a design decision, which I knew would cause questions but which I firmly believe in. So I think the opposite is true in general. It's also worth noting that (*) E.g. use the command-loop, instead of the previous primitive I strongly believe in "accomplish more by not implementing unnecessary features". Unfortunately that is not always possible with packages as popular as Magit; there are to many users having an opinion on what feature is necessary. But that's okay, Magit's success is also in part due to my willingness to implement features I do not firmly believe in. (In some cases I stick to that opinion, in others I come around once I got a change to play with that feature because I took the time to implement it.) What I am trying to say is that Except for the isearch thing the "features" that came for free in For example the "change the log arguments in the current log or status buffer" popup/transient cannot be used in non-log/status buffers, but if the user is free to change the buffer after the popup/transient is activated, then that by-passes the "is this a valid buffer" check that the transient prefix commands runs before actually activating the transient state. When users are allowed to run arbitrary commands, then they can change what buffer is current and if they then invokes a suffix command that expects that the current buffer is a valid buffer, then all kinds of things can happen. I firmly believe that transients as implemented by I can understand that people who have only used But Users noticed and made use of that "feature". I didn't remove it because I knew that would break the workflow of some users. But when it came to implement
I cannot parse that sentence, but I think you are asking "Would it be possible to add an option that would allow me to opt-in to being able to invoke any command while a transient is active?". To which the answer is: I believe so, it would involve setting the The purpose of the (define-transient-command outline-navigate ()
:transient-suffix 'transient--do-stay
:transient-non-suffix 'transient--do-warn
[("p" "next visible heading" outline-previous-visible-heading)
("n" "next visible heading" outline-next-visible-heading)]) For such a transient it makes sense to allow other commands also. |
Arrow key navigation, mouse support, and isearch support have been restored. You need to set an option to enable these features. See #42. |
In case someone still prefers the old behavior of Note: setting I have been using transient only for several days, so there may be some problems that I have not noticed yet, but the following config works for me so far: (with-eval-after-load 'transient
(setq
transient--buffer-name "*transient*"
;; transient-detect-key-conflicts t
;; transient-highlight-mismatched-keys t
;; transient--debug t
transient-enable-popup-navigation t
transient-mode-line-format mode-line-format
transient-display-buffer-action '(display-buffer-below-selected))
(let ((map transient-base-map))
(define-key map (kbd "C-g") 'transient-quit-all)
(define-key map (kbd "C-q") 'transient-quit-one)
(define-key map (kbd "DEL") 'transient-quit-one))
(define-key transient-map (kbd "C-h") nil)
(let ((map transient-popup-navigation-map))
(define-key map (kbd "<tab>") 'transient-forward-button)
(define-key map (kbd "<backtab>") 'transient-backward-button ))
(transient-suffix-put 'transient-common-commands
"C-g" :command 'transient-quit-all)
(transient-suffix-put 'transient-common-commands
"C-q" :command 'transient-quit-one)
(defun al/transient-fix-window ()
"Return `transient--window' to a 'normal' state."
(set-window-dedicated-p transient--window nil)
(set-window-parameter transient--window 'no-other-window nil)
(with-selected-window transient--window
(setq
window-size-fixed nil
cursor-in-non-selected-windows t
cursor-type (default-value 'cursor-type)
mode-line-buffer-identification
(list ""
(symbol-name (oref transient--prefix command))
" " (default-value 'mode-line-buffer-identification)))))
(define-derived-mode al/transient-mode special-mode "al/transient"
(setq buffer-read-only nil)
(al/transient-fix-window))
(defun al/transient-push-keymap (map)
(with-demoted-errors "al/transient-push-keymap: %S"
(internal-push-keymap (symbol-value map) 'al/transient-mode-map)))
(defun al/transient-pop-keymap (map)
(with-demoted-errors "al/transient-pop-keymap: %S"
(internal-pop-keymap (symbol-value map) 'al/transient-mode-map)))
(defun al/transient-fix-show (&rest _)
(transient--debug 'al/transient-fix-show)
(al/transient-fix-window)
(select-window transient--window))
(defun al/transient-fix-init (&rest _)
(transient--debug 'al/transient-fix-init)
(with-current-buffer transient--buffer-name
(al/transient-mode)))
(defun al/transient-fix-pre/post-command (fun &rest args)
(transient--debug 'al/transient-fix-pre/post-command)
;; Do anything only for transient commands.
(when (or (get this-command 'transient--prefix)
(string-match-p "\\`transient"
(symbol-name this-command))
(and transient--transient-map
(string= (buffer-name) transient--buffer-name)
(lookup-key transient--transient-map
(this-single-command-raw-keys))))
(apply fun args)))
(defun al/transient-fix-delete-window (fun &rest args)
(unless (eq transient--exitp 'suspend)
(apply fun args)))
(advice-add 'transient--minibuffer-setup :override #'ignore)
(advice-add 'transient--minibuffer-exit :override #'ignore)
(advice-add 'transient--push-keymap :override #'al/transient-push-keymap)
(advice-add 'transient--pop-keymap :override #'al/transient-pop-keymap)
(advice-add 'transient--pre-command :around #'al/transient-fix-pre/post-command)
(advice-add 'transient--post-command :around #'al/transient-fix-pre/post-command)
(advice-add 'transient--show :after #'al/transient-fix-show)
(advice-add 'transient--init-transient :after #'al/transient-fix-init)
(advice-add 'transient--delete-window :around #'al/transient-fix-delete-window)) |
I was trying to look for previous issues, apologies if I missed some.
magit-version: Magit 20190215.1903, Git 2.17.1, Emacs 25.2.2, gnu/linux
I upgraded today and saw that the popup buffer behaves differently (I guess with the switch to transient?). My small nit is that it takes ownership too much. Concretely it would be nice if it did not hijack all my usual emacs bindings, especially the
C-x
bindings, and the windmoveshift-{left,up,right,down}
keys to move between windows.The other small nit: it was great being able to treat the popup as a normal buffer, where I could enter and do
C-s
to search quickly for a command instead of reading through all of them to find the one I need.Finally, can you give me a quick pointer how to restore
q
as an alias forC-g
to quit magit popups? At the moment,q
still works for e.g. log, stash, revision buffers, but not the popups.As always, thanks for all the great work!
The text was updated successfully, but these errors were encountered: