Adam Spiers <[email protected]> writes: > On Thu, Aug 29, 2013 at 12:31:41PM -0400, Tom Davis wrote: >> [email protected] writes: >> > On Thu, Aug 29, 2013 at 10:13:31AM -0400, Tom Davis wrote: >> >> [email protected] writes: >> >> > On 27 August 2013 14:40, Phil Jackson <[email protected]> wrote: >> >> >> Hey all, >> >> >> >> >> >> When I wrote the current menu-popup system in Magit I implemented >> >> >> a popup for commiting just like the one that's been built >> >> >> here. People were generally happy with the popup menus elsewhere >> >> >> but I pulled it out of commit because there was so much >> >> >> resistance. >> >> >> >> >> >> Before I quit Magit I had intended to keep the current workflow >> >> >> but have a slightly nicer implementation of the >> >> >> header-for-options system that served us reasonably well for all >> >> >> this time. >> >> >> >> >> >> My two cents; I'm not sure I like the new way. I find the menu is >> >> >> obstructive in this instance. >> >> > >> >> > Agreed Phil. The most common commit use case by far is surely "commit >> >> > without needing any special options", so to always pop up a menu >> >> > offering options is suboptimal for the majority of cases. Presumably >> >> > this is why there was so much resistance when you first introduced it. >> >> >> >> Overall I'd call it a great addition despite the annoyance of having to >> >> hit "c c" because, for the times when I *did* need options, I simply >> >> couldn't use magit at all for that commit. >> > >> > Why not? >> >> Because when you need to pass "-n" to "git commit" and you can't, you >> can't commit. > > I see. I never needed that personally, but totally understand ;-) > >> >> There are definitely still >> >> bugs (first line of commit message is in the wrong place, the commit >> >> buffer doesn't close when the commit finishes, etc.) but all of these >> >> are minor compared with the use cases enabled by the new commit >> >> mechanism. >> > >> > Which new use cases? I guess there are some which I missed. But even >> > so, that does not in itself justify introducing a popup menu, because >> > surely the new use cases could have been added to the old style of >> > interface? >> >> Perhaps, but the header-style interface to commits was different from >> all other commands that include options, AFAIK. > > True, but we shouldn't let the quest for consistency get in the way of > the quest for the best interface :-) > >> Discovery is also a concern; if you add, e.g., "C-c C-n" as another >> commit header thingy, the only way to find that would be via the >> mode help. Hell, maybe it did get added and I never knew ;) > > That's a very good point. However, it should be easy to discover such > things by reading the manual, and prioritising catering for newbies > over catering for experienced users is a slippery slope. Almost by > definition, anyone who uses git + emacs + magit is an advanced user > who is likely to invest time in learning their tools properly ;-) > > I think the most telling evidence is that Phil mentioned he already > added a popup menu for commit in the past, but then had to withdraw it > due to large resistance. Surely we can learn from previous > experiences? > > That said, the ideal world would be a M-x customize-option allowing a > choice between `c' immediately progressing to the commit buffer, or > popping up a menu. If that existed, I wouldn't really mind what the > default setting was :-) This is very similar to the request here: >
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.
-- Eric Schulte https://cs.unm.edu/~eschulte PGP: 0x614CA05D
>From 352addd446647515cf0f041c0bad3a87c4b36e90 Mon Sep 17 00:00:00 2001 From: Eric Schulte <[email protected]> Date: Thu, 29 Aug 2013 13:14:25 -0600 Subject: [PATCH] only show action options when prefix argument used This commit also introduces the `magit-confirmation-groups' customization variable which may be used to control which groups require explicit confirmation. Only pushing requires explicit confirmation by default. Not really tested, but worked as expected for this commit. --- magit-key-mode.el | 44 ++++++++++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/magit-key-mode.el b/magit-key-mode.el index 3932085..36a31e1 100644 --- a/magit-key-mode.el +++ b/magit-key-mode.el @@ -667,6 +667,18 @@ Return the point before the actions part, if any, nil otherwise." p)) ;;; Generate Groups +(defcustom magit-confirmation-groups '(pushing) + "Actions which will require explicit confirmation." + :group 'git-commit + :type 'list + :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)))) (defun magit-key-mode-de-generate (group) "Unbind the function for GROUP." @@ -677,19 +689,27 @@ Return the point before the actions part, if any, nil otherwise." "Generate the key-group menu for GROUP." (let ((opts (magit-key-mode-options-for-group group))) (eval - `(defun ,(intern (concat "magit-key-mode-popup-" (symbol-name group))) nil + `(defun ,(intern (concat "magit-key-mode-popup-" (symbol-name group))) + (&optional arg) ,(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)))))))) + (interactive "P") + (if 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 first action directly + ,(let ((action (cddadr (assoc 'actions opts)))) + (if (member group magit-confirmation-groups) + `(when (y-or-n-p (format "run %a" ,group)) ,action) + action))))))) ;; create the interactive functions for the key mode popups (which are ;; applied in the top-level key maps) -- 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.
