Óscar Fuentes <[email protected]> writes: > Eric Schulte <[email protected]> > writes: > >>> Why not have `c' open the commit buffer directly, and `c' with a prefix >>> argument (i.e. `C-u c') could open up the full option menu. In fact I >>> would recommend this as the default for *all* magit commands (dangerous >>> commands would probably want to be wrapped in a y-or-n-p). >>> >>> This would be efficient and convenient, provide for internal consistency >>> within magit and this would be consistent with how Emacs at large >>> handles optional command customization. To new users who have used >>> Emacs before this should be the obvious thing to try. >>> >>> If there is any interest I'd be happy to put together a patch, given >>> magit's very nice meta-programmed action mechanisms it seems like it >>> should be a small change... very easy to implement, attached. >>> >> >> I'm happy to hear that something along these lines will be adopted. >> >> Having used my previous patch for an hour or so I'm finding this >> prefix-argument tact matches very well with my own personal Emacs-warped >> muscle memory. If it's useful I'm attaching an improved version of the >> previous patch. > > I strongly oppose this change. For committing 99% of the time you want > what now is `c c' but for other commands (log, branch) it is common to > use a command variant (actually, `b' is a gateway for a group of > different commands.) Pressing `C-u b v' for entering the branch manager > or `C-u l L' for Long Log is anything but efficient. The key-groups > design already allowed for the "most common variant" by assigning it the > same letter as the key-group trigger (`l l', `b b', etc.) This > introduced a negligible delay in exchange for lots of flexibility and > grow space. > > Seriously, I can't see what's so inefficient with pressing an extra `c' > for committing. Two tenths of a second for an operation that culminates > a task that typically takes a minimum of several minutes? C'mon! > > It is true that `c c' faced enough resistance to ditch it. But the whole > key-groups feature faced that same resistance and it survived, simply > because Magit was running out of keys and there was a nightmare of > command-prefix tricks (uh!) for accessing just a few of the most common > options (all the rest be damned, as well as their combinations.) It > could be said that `c c' was reverted to plain `c' just because > committing has no variants (as logging does) and the options for `git > commit' were not popular enough.
Fair enough. While I disagree that pressing command keys twice is negligible, and I spend no where near several minutes writing my commit messages, we probably just have different commit styles. Perhaps a workable middle ground acknowledging such personal differences would be to introduce a variable (say `magit-immediate-commands') which controls which commands run immediately by default, and all other commands could open the option dialog. Calling any command with a prefix argument could then do the opposite of the default (e.g., open option pane or run immediately). This variable could initially just be set to '(committing) so that new users are exposed to the options. The attached patch implements such a solution. In addition it changes the default action from "first listed" to "one with the same key stroke as the action". Please try it out and let me know what you think.
BTW: for those not familiar with the email/patch workflow, if you save the attached patch to a file, you can then apply it to a repository by running the following from within your git directory. git am /tmp/0001-patch-file-name Cheers, -- Eric Schulte https://cs.unm.edu/~eschulte PGP: 0x614CA05D
>From 6685f8a5f3fae3f9cf940d90027d2041e92e57fd Mon Sep 17 00:00:00 2001 From: Eric Schulte <[email protected]> Date: Thu, 29 Aug 2013 13:14:25 -0600 Subject: [PATCH] customizable display of options to magit commands Commands from `magit-immediate-commands' will be run immediately without displaying options first, all other magit commands are run as normal. With a prefix argument these behavior reverse with members of `magit-immediate-commands' displaying options and other commands running immediately. Whenever a command is immediately run, the action taken is that which starts with the same letter as that used to invoke the command. When `magit-immediate-commands' is set through the customization interface, all related magit command functions are redefined with the appropriate behavior, argument list and documentation. --- magit-key-mode.el | 68 ++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 50 insertions(+), 18 deletions(-) diff --git a/magit-key-mode.el b/magit-key-mode.el index 272976b..1ef81ef 100644 --- a/magit-key-mode.el +++ b/magit-key-mode.el @@ -625,7 +625,6 @@ Return the point before the actions part, if any, nil otherwise." p)) ;;; Generate Groups - (defun magit-key-mode-de-generate (group) "Unbind the function for GROUP." (fmakunbound @@ -633,27 +632,60 @@ Return the point before the actions part, if any, nil otherwise." (defun magit-key-mode-generate (group) "Generate the key-group menu for GROUP." - (let ((opts (magit-key-mode-options-for-group group))) + (let* ((opts (magit-key-mode-options-for-group group)) + (immediate (member group magit-immediate-commands)) + (arg (if immediate 'options 'no-options))) (eval - `(defun ,(intern (concat "magit-key-mode-popup-" (symbol-name group))) nil - ,(concat "Key menu for " (symbol-name group)) - (interactive) - (magit-key-mode - (quote ,group) - ;; As a tempory kludge it is okay to do this here. - ,(cl-case group - (logging - '(when magit-have-graph - (list "--graph"))) - (diff-options - '(when (local-variable-p 'magit-diff-options) - magit-diff-options)))))))) + `(defun ,(intern (concat "magit-key-mode-popup-" (symbol-name group))) + (&optional ,arg) + ,(if immediate + (format "Run %s.\nWith prefix argument show %s Key menu." + group group) + (format "Key menu for %s.\nWith prefix argument run %s directly." + group group)) + (interactive "P") + (message "showing options %s" ,(if immediate arg `(not ,arg))) + (if ,(if immediate arg `(not ,arg)) + ;; pop up the full menu + (magit-key-mode + (quote ,group) + ;; As a tempory kludge it is okay to do this here. + ,(cl-case group + (logging + '(when magit-have-graph + (list "--graph"))) + (diff-options + '(when (local-variable-p 'magit-diff-options) + magit-diff-options)))) + ;; execute the action with the same key binding directly + (let ((keys (this-command-keys))) + (call-interactively + (third (assoc (subseq keys (1- (length keys))) + ',(cdr (assoc 'actions opts))))))))))) ;; create the interactive functions for the key mode popups (which are ;; applied in the top-level key maps) -(mapc (lambda (g) - (magit-key-mode-generate (car g))) - magit-key-mode-groups) +(defcustom magit-immediate-commands '(committing) + "Actions listed here are run immediately instead of displaying options. +The options for these commands may still be viewed by invoking +the commands with a prefix argument. Values should be keys from +`magit-key-mode-groups'. When this variable is set action +functions will be automatically regenerated using +`magit-key-mode-generate'." + :group 'git-commit + :type `(repeat + (choice + ,@(mapcar (lambda (g) + `(const :tag ,(symbol-name (car g)) ,(car g))) + magit-key-mode-groups))) + :initialize + (lambda (symbol value) + (let ((val (eval value))) + ;; set the value + (set-default symbol val) + ;; update group functions + (mapc (lambda (g) (magit-key-mode-generate (car g))) + magit-key-mode-groups)))) (provide 'magit-key-mode) ;;; magit-key-mode.el ends here -- 1.8.4
-- --- You received this message because you are subscribed to the Google Groups "magit" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/groups/opt_out.
