* support for git commit --amend/--signoff @ 2010-06-11 6:19 Dan Nicolaescu 2010-06-11 8:09 ` Juri Linkov 0 siblings, 1 reply; 38+ messages in thread From: Dan Nicolaescu @ 2010-06-11 6:19 UTC (permalink / raw) To: emacs-devel It would be nice if VC commit would support --amend and --signoff for git. The patch below adds key bindings to toggle some variables in the log-edit buffer. (for --amend it also inserts the contents of the last commit message) Now vc-git-checkin needs to know any of these variables has been set, so that it can pass the right flags to the git commit command. Other VC backends might want to implement similar things. What is the best way to pass custom flags to the vc-checkin command? === modified file 'lisp/vc-git.el' --- lisp/vc-git.el 2010-06-09 05:24:01 +0000 +++ lisp/vc-git.el 2010-06-10 00:00:50 +0000 @@ -550,6 +550,48 @@ or an empty string if none." (declare-function log-edit-extract-headers "log-edit" (headers string)) +(defvar vc-git-log-edit-signoff nil) + +(defvar vc-git-log-edit-amend nil) + +(defun vc-git-log-edit-toggle-signoff () + (interactive) + (setq vc-git-log-edit-signoff (not vc-git-log-edit-signoff))) + +(defun vc-git-log-edit-toggle-amend () + (interactive) + (unless vc-git-log-edit-amend + (vc-git-command (current-buffer) 1 nil + "log" "--max-count=1" "--pretty=format:%s" "HEAD")) + (setq vc-git-log-edit-amend (not vc-git-log-edit-amend))) + +(defvar vc-git-log-edit-map + (let ((map (make-sparse-keymap "Git-Log-Edit")) + (menu-map (make-sparse-keymap))) + + ;; FIXME: Are these key bindings OK? + (define-key map "\C-c\C-s" 'vc-git-log-edit-toggle-signoff) + (define-key map "\C-c\C-m" 'vc-git-log-edit-toggle-amend) + + ;; FIXME: This does not work. And it would be better if we could + ;; add items to the Log-Edit menu. + (define-key map [menu-bar vc-git-log-edit] (cons "Git-Log-Edit" menu-map)) + (define-key menu-map [ts] + '(menu-item "Signoff" vc-git-log-edit-toggle-signoff + :help "Toggle signoff" + :button (:toggle . vc-git-log-edit-signoff))) + (define-key menu-map [ta] + '(menu-item "Amend" vc-git-log-edit-toggle-amend + :help "Toggle amend, insert old log when turning it on" + :button (:toggle . vc-git-log-edit-amend))) + map)) + +(define-derived-mode vc-git-log-edit-mode log-edit-mode "*VC-log*" + "Major mode for editing Git log messages. +It is based on `log-edit-mode', it has Git specific extensions." + (make-local-variable 'vc-git-log-edit-signoff) + (make-local-variable 'vc-git-log-edit-amend)) + (defun vc-git-checkin (files rev comment) (let ((coding-system-for-write vc-git-commits-coding-system)) (apply 'vc-git-command nil 0 files ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-11 6:19 support for git commit --amend/--signoff Dan Nicolaescu @ 2010-06-11 8:09 ` Juri Linkov 2010-06-11 13:23 ` Dan Nicolaescu 0 siblings, 1 reply; 38+ messages in thread From: Juri Linkov @ 2010-06-11 8:09 UTC (permalink / raw) To: Dan Nicolaescu; +Cc: emacs-devel > It would be nice if VC commit would support --amend and --signoff for git. > > The patch below adds key bindings to toggle some variables in the log-edit buffer. > (for --amend it also inserts the contents of the last commit message) > > Now vc-git-checkin needs to know any of these variables has been set, > so that it can pass the right flags to the git commit command. > > Other VC backends might want to implement similar things. > > What is the best way to pass custom flags to the vc-checkin command? The same way as you pass Author:, Summary: and Fixes:? BTW, Author:, Summary: and Fixes: need key bindings too. What about using the same key prefix as message-mode - `C-c C-f'? I.e. like `C-c C-f C-s' (`message-goto-subject') to add `C-c C-f C-s' (`log-edit-goto-subject'), etc. -- Juri Linkov http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-11 8:09 ` Juri Linkov @ 2010-06-11 13:23 ` Dan Nicolaescu 2010-06-11 14:18 ` Stefan Monnier 0 siblings, 1 reply; 38+ messages in thread From: Dan Nicolaescu @ 2010-06-11 13:23 UTC (permalink / raw) To: Juri Linkov; +Cc: emacs-devel Juri Linkov <juri@jurta.org> writes: >> It would be nice if VC commit would support --amend and --signoff for git. >> >> The patch below adds key bindings to toggle some variables in the log-edit buffer. >> (for --amend it also inserts the contents of the last commit message) >> >> Now vc-git-checkin needs to know any of these variables has been set, >> so that it can pass the right flags to the git commit command. >> >> Other VC backends might want to implement similar things. >> >> What is the best way to pass custom flags to the vc-checkin command? > > The same way as you pass Author:, Summary: and Fixes:? Those are done by parsing the contents of the *VC-log* buffer. For --amend I think we should have a different mechanism that does not involve editing the text in the *VC-log* buffer, it's a more destructive operation and it's not a good idea to allow the user to do it by mistake by just placing an Amend: line in the buffer. We need a different mechanism. I added one such a mechanism at some point, an optional `extra' argument to the vc-BACKEND-checkin command, but that got removed... > > BTW, Author:, Summary: and Fixes: need key bindings too. > What about using the same key prefix as message-mode - `C-c C-f'? > I.e. like `C-c C-f C-s' (`message-goto-subject') > to add `C-c C-f C-s' (`log-edit-goto-subject'), etc. > > -- > Juri Linkov > http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-11 13:23 ` Dan Nicolaescu @ 2010-06-11 14:18 ` Stefan Monnier 2010-06-11 16:14 ` Štěpán Němec ` (2 more replies) 0 siblings, 3 replies; 38+ messages in thread From: Stefan Monnier @ 2010-06-11 14:18 UTC (permalink / raw) To: Dan Nicolaescu; +Cc: Juri Linkov, emacs-devel > For --amend I think we should have a different mechanism that does not > involve editing the text in the *VC-log* buffer, it's a more > destructive operation and it's not a good idea to allow the user to do > it by mistake by just placing an Amend: line in the buffer. You might be right, but I don't think it would be terribly bad to have it be editable in the buffer (after all it's also just an "--amend" away when committing on the command line). I'm not sure what UI you're considering to use and what users would want, because I've never used Git's amend feature. But I think it should be considered in the light of similar functionality in other VCS, such as maybe "cvs admin -m" and DaRCS's "amend-record". For PCL-CVS what I offered was a way to edit a log-message (typically called from the log-view buffer). But admittedly, "cvs admin -m" is a fairly different beast from Git's amend. If we only consider Git's and DaRCS's forms of amend, I'd say that the "Amend:" header might be a good approach, and that it should specify the revision/patch that's amended. So for Git, you could have a command that inserts "Amend: <SHA-1>" and then the backend could check that the SHA-1 is the right one (which would avoid accidental use). Stefan ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-11 14:18 ` Stefan Monnier @ 2010-06-11 16:14 ` Štěpán Němec 2010-06-11 20:26 ` Stefan Monnier 2010-06-11 17:34 ` Dan Nicolaescu 2010-06-11 19:27 ` Juri Linkov 2 siblings, 1 reply; 38+ messages in thread From: Štěpán Němec @ 2010-06-11 16:14 UTC (permalink / raw) To: Stefan Monnier; +Cc: Juri Linkov, Dan Nicolaescu, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: > If we only consider Git's and DaRCS's forms of amend, I'd say that the > "Amend:" header might be a good approach, and that it should specify the > revision/patch that's amended. So for Git, you could have a command > that inserts "Amend: <SHA-1>" and then the backend could check that the > SHA-1 is the right one (which would avoid accidental use). No idea about Darcs, but `git commit --amend' always changes the tip of the current branch, you can't specify another commit to amend; so the above would not be useful in this case (same for --signoff). Štěpán ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-11 16:14 ` Štěpán Němec @ 2010-06-11 20:26 ` Stefan Monnier 2010-06-12 2:19 ` Dan Nicolaescu 0 siblings, 1 reply; 38+ messages in thread From: Stefan Monnier @ 2010-06-11 20:26 UTC (permalink / raw) To: Štěpán Němec; +Cc: Juri Linkov, Dan Nicolaescu, emacs-devel >> If we only consider Git's and DaRCS's forms of amend, I'd say that the >> "Amend:" header might be a good approach, and that it should specify the >> revision/patch that's amended. So for Git, you could have a command >> that inserts "Amend: <SHA-1>" and then the backend could check that the >> SHA-1 is the right one (which would avoid accidental use). > No idea about Darcs, but `git commit --amend' always changes the tip of > the current branch, you can't specify another commit to amend; I know that (and DaRCS doesn't have such a limitation). > so the above would not be useful in this case (same for --signoff). It is not useful but it is needed because an empty header is normally the same as no header, so just "Amend:" can't be enough, we'd have to put something there. I suggested a SHA-1 just because Dan though there was a risk of people writing the header by mistake. Stefan ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-11 20:26 ` Stefan Monnier @ 2010-06-12 2:19 ` Dan Nicolaescu 2010-06-12 19:59 ` Juri Linkov 2010-06-12 20:19 ` Stefan Monnier 0 siblings, 2 replies; 38+ messages in thread From: Dan Nicolaescu @ 2010-06-12 2:19 UTC (permalink / raw) To: Stefan Monnier; +Cc: Juri Linkov, Štěpán Němec, emacs-devel Stefan Monnier <monnier@IRO.UMontreal.CA> writes: >>> If we only consider Git's and DaRCS's forms of amend, I'd say that the >>> "Amend:" header might be a good approach, and that it should specify the >>> revision/patch that's amended. So for Git, you could have a command >>> that inserts "Amend: <SHA-1>" and then the backend could check that the >>> SHA-1 is the right one (which would avoid accidental use). > >> No idea about Darcs, but `git commit --amend' always changes the tip of >> the current branch, you can't specify another commit to amend; > > I know that (and DaRCS doesn't have such a limitation). >> so the above would not be useful in this case (same for --signoff). > > It is not useful but it is needed because an empty header is normally > the same as no header, so just "Amend:" can't be enough, we'd have to > put something there. I suggested a SHA-1 just because Dan though there > was a risk of people writing the header by mistake. That would be a very strong argument agains doing it that way for amend then. Coupled with the fact that for amend we actually want to insert the previous comit log, that calls for a different solution. We could have a log-edit-extra-flags function that computes a set of extra flags, and pass those flags to vc-git-checkin. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-12 2:19 ` Dan Nicolaescu @ 2010-06-12 19:59 ` Juri Linkov 2010-06-12 20:19 ` Stefan Monnier 1 sibling, 0 replies; 38+ messages in thread From: Juri Linkov @ 2010-06-12 19:59 UTC (permalink / raw) To: Dan Nicolaescu; +Cc: Stefan Monnier, emacs-devel > We could have a log-edit-extra-flags function that computes a set of > extra flags, and pass those flags to vc-git-checkin. Is it like log-edit-extract-headers? Instead of extracting headers it could extract variables. -- Juri Linkov http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-12 2:19 ` Dan Nicolaescu 2010-06-12 19:59 ` Juri Linkov @ 2010-06-12 20:19 ` Stefan Monnier 2010-06-19 6:38 ` Dan Nicolaescu 1 sibling, 1 reply; 38+ messages in thread From: Stefan Monnier @ 2010-06-12 20:19 UTC (permalink / raw) To: Dan Nicolaescu; +Cc: Juri Linkov, Štěpán Němec, emacs-devel > We could have a log-edit-extra-flags function that computes a set of > extra flags, and pass those flags to vc-git-checkin. If amend is triggered from a new command (like vc-amend), then there's no need for any special support in log-edit. We could easily store the relevant info in log-edit-callback or in vc-specific buffer-local variables. Stefan ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-12 20:19 ` Stefan Monnier @ 2010-06-19 6:38 ` Dan Nicolaescu 2010-06-23 7:17 ` Stefan Monnier 0 siblings, 1 reply; 38+ messages in thread From: Dan Nicolaescu @ 2010-06-19 6:38 UTC (permalink / raw) To: Stefan Monnier; +Cc: Juri Linkov, Štěpán Němec, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> We could have a log-edit-extra-flags function that computes a set of >> extra flags, and pass those flags to vc-git-checkin. > > If amend is triggered from a new command (like vc-amend), then there's no > need for any special support in log-edit. We could easily store the > relevant info in log-edit-callback or in vc-specific > buffer-local variables. The problem with that is that vc-amend does not integrate very well with C-x v v and it is very Git specific, and given the reluctance of some other VCs to change history, it does not seem that it will be generalized. More, it does not solve the problem of --signoff (which might be adopted by other VCs). Here's a solution that takes care of all problems and it is extensible. I adds one line to log-edit.el and two to vc.el. vc-git-log-edit-get-extra-flags-function does the work in vc-git.el === modified file 'lisp/vc/log-edit.el' --- lisp/vc/log-edit.el 2010-06-12 17:14:43 +0000 +++ lisp/vc/log-edit.el 2010-06-16 17:37:47 +0000 @@ -189,6 +189,7 @@ when this variable is set to nil.") (defvar log-edit-callback nil) (defvar log-edit-diff-function nil) (defvar log-edit-listfun nil) +(defvar log-edit-get-extra-flags-function nil) (defvar log-edit-parent-buffer nil) === modified file 'lisp/vc/vc.el' --- lisp/vc/vc.el 2010-06-11 19:09:57 +0000 +++ lisp/vc/vc.el 2010-06-16 19:09:37 +0000 @@ -1398,7 +1406,9 @@ Runs the normal hooks `vc-before-checkin ;; vc-checkin-switches, but 'the' local buffer is ;; not a well-defined concept for filesets. (progn - (vc-call-backend backend 'checkin files rev comment) + (vc-call-backend backend 'checkin files rev comment + (when log-edit-get-extra-flags-function + (funcall log-edit-get-extra-flags-function))) (mapc 'vc-delete-automatic-version-backups files)) `((vc-state . up-to-date) (vc-checkout-time . ,(nth 5 (file-attributes file))) === modified file 'lisp/vc/vc-annotate.el' === modified file 'lisp/vc/vc-bzr.el' === modified file 'lisp/vc/vc-git.el' --- lisp/vc/vc-git.el 2010-06-11 19:09:57 +0000 +++ lisp/vc/vc-git.el 2010-06-16 18:10:00 +0000 @@ -550,13 +550,65 @@ or an empty string if none." (declare-function log-edit-extract-headers "log-edit" (headers string)) -(defun vc-git-checkin (files rev comment) +(defvar vc-git-log-edit-signoff nil) + +(defvar vc-git-log-edit-amend nil) + +(defun vc-git-log-edit-toggle-signoff () + (interactive) + (setq vc-git-log-edit-signoff (not vc-git-log-edit-signoff))) + +(defun vc-git-log-edit-toggle-amend () + (interactive) + (unless vc-git-log-edit-amend + (vc-git-command (current-buffer) 1 nil + "log" "--max-count=1" "--pretty=format:%s" "HEAD")) + (setq vc-git-log-edit-amend (not vc-git-log-edit-amend))) + +(defvar vc-git-log-edit-map + (let ((map (make-sparse-keymap "Git-Log-Edit")) + (menu-map (make-sparse-keymap))) + + ;; FIXME: Are these key bindings OK? + (define-key map "\C-c\C-s" 'vc-git-log-edit-toggle-signoff) + (define-key map "\C-c\C-m" 'vc-git-log-edit-toggle-amend) + + ;; FIXME: This does not work. And it would be better if we could + ;; add items to the Log-Edit menu. + (define-key map [menu-bar vc-git-log-edit] (cons "Git-Log-Edit" menu-map)) + (define-key menu-map [ts] + '(menu-item "Signoff" vc-git-log-edit-toggle-signoff + :help "Toggle signoff" + :button (:toggle . vc-git-log-edit-signoff))) + (define-key menu-map [ta] + '(menu-item "Amend" vc-git-log-edit-toggle-amend + :help "Toggle amend, insert old log when turning it on" + :button (:toggle . vc-git-log-edit-amend))) + map)) + +(define-derived-mode vc-git-log-edit-mode log-edit-mode "*VC-log*" + "Major mode for editing Git log messages. +It is based on `log-edit-mode', it has Git specific extensions." + (set (make-local-variable 'log-edit-get-extra-flags-function) + 'vc-git-log-edit-get-extra-flags-function) + (make-local-variable 'vc-git-log-edit-signoff) + (make-local-variable 'vc-git-log-edit-amend)) + +(defun vc-git-log-edit-get-extra-flags-function () + (let (result) + (when vc-git-log-edit-amend + (push "--amend" result)) + (when vc-git-log-edit-signoff + (push "--signoff" result)))) + +(defun vc-git-checkin (files rev comment &optional extra-flags) (let ((coding-system-for-write vc-git-commits-coding-system)) (apply 'vc-git-command nil 0 files (nconc (list "commit" "-m") (log-edit-extract-headers '(("Author" . "--author") ("Date" . "--date")) comment) + extra-flags (list "--only" "--"))))) (defun vc-git-find-revision (file rev buffer) ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-19 6:38 ` Dan Nicolaescu @ 2010-06-23 7:17 ` Stefan Monnier 2010-06-23 7:45 ` David Kastrup ` (2 more replies) 0 siblings, 3 replies; 38+ messages in thread From: Stefan Monnier @ 2010-06-23 7:17 UTC (permalink / raw) To: Dan Nicolaescu; +Cc: Juri Linkov, Štěpán Němec, emacs-devel >>> We could have a log-edit-extra-flags function that computes a set of >>> extra flags, and pass those flags to vc-git-checkin. >> If amend is triggered from a new command (like vc-amend), then there's no >> need for any special support in log-edit. We could easily store the >> relevant info in log-edit-callback or in vc-specific >> buffer-local variables. > The problem with that is that vc-amend does not integrate very well > with C-x v v That doesn't bother me too much. I wish we could move further away from C-x v v since I find it doesn't really work for modern VCSes. > and it is very Git specific, Not sure about the "very", but in any case I don't see why that would argue for integration inside "commit" rather than for the addition of a separate command. > and given the reluctance of some other VCs to change history, it does > not seem that it will be generalized. DaRCS already supports a more general form of Git's amend. Bazaar is unlikely to go there, tho, indeed (although they are not opposed to adding a more limited command to modify commit-comments). Don't know about others. > More, it does not solve the problem of --signoff (which might be > adopted by other VCs). What's the problem with sign-off? It seems this one fits perfectly well inside a "Signed-Off-By:" header. > Here's a solution that takes care of all problems and it is extensible. > I adds one line to log-edit.el and two to vc.el. > vc-git-log-edit-get-extra-flags-function does the work in vc-git.el Why add a var "log-edit-get-extra-flags-function" that's never used by log-edit? BTW, I think you can get what you want already without touching any of vc.el and log-edit.el by making vc-git-log-edit-toggle-amend tweak the log-edit-callback variable. Maybe adding an `extra-flags' arg to `checkin' is the better approach, but adding it everywhere just for the sake of Git doesn't sound too good, which is why I'm resisting it. Also as a user I'd *really* like to have a clear visual feedback about the fact that I'm amending something, which is another reason why having it inside the headers is an attractive direction. Stefan ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-23 7:17 ` Stefan Monnier @ 2010-06-23 7:45 ` David Kastrup 2010-06-23 9:00 ` Miles Bader 2010-06-23 18:45 ` Dan Nicolaescu 2 siblings, 0 replies; 38+ messages in thread From: David Kastrup @ 2010-06-23 7:45 UTC (permalink / raw) To: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >>>> We could have a log-edit-extra-flags function that computes a set of >>>> extra flags, and pass those flags to vc-git-checkin. >>> If amend is triggered from a new command (like vc-amend), then there's no >>> need for any special support in log-edit. We could easily store the >>> relevant info in log-edit-callback or in vc-specific >>> buffer-local variables. >> The problem with that is that vc-amend does not integrate very well >> with C-x v v > > That doesn't bother me too much. I wish we could move further away from > C-x v v since I find it doesn't really work for modern VCSes. > >> and it is very Git specific, > > Not sure about the "very", but in any case I don't see why that would > argue for integration inside "commit" rather than for the addition of > a separate command. I thing that a zero prefix argument to C-x v v would feel quite natural for "amend". -- David Kastrup ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-23 7:17 ` Stefan Monnier 2010-06-23 7:45 ` David Kastrup @ 2010-06-23 9:00 ` Miles Bader 2010-06-23 18:55 ` Dan Nicolaescu 2010-06-23 18:45 ` Dan Nicolaescu 2 siblings, 1 reply; 38+ messages in thread From: Miles Bader @ 2010-06-23 9:00 UTC (permalink / raw) To: Stefan Monnier Cc: Juri Linkov, Dan Nicolaescu, Štěpán Němec, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> The problem with that is that vc-amend does not integrate very well >> with C-x v v > > That doesn't bother me too much. I wish we could move further away from > C-x v v since I find it doesn't really work for modern VCSes. It doesn't...? I mean, for single-file checkins[*], it works fine, right? [*] Maybe rare with Emacs, because it uses changelogs, but fairly common, IME, in projects that don't, especially if you use a "commit really often" style I do admit, I'd like some better frobs to do multiple-file commits without bringing up a vc-dir... -Miles -- `...the Soviet Union was sliding in to an economic collapse so comprehensive that in the end its factories produced not goods but bads: finished products less valuable than the raw materials they were made from.' [The Economist] ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-23 9:00 ` Miles Bader @ 2010-06-23 18:55 ` Dan Nicolaescu 0 siblings, 0 replies; 38+ messages in thread From: Dan Nicolaescu @ 2010-06-23 18:55 UTC (permalink / raw) To: Miles Bader Cc: Juri Linkov, Štěpán Němec, Stefan Monnier, emacs-devel Miles Bader <miles@gnu.org> writes: > I do admit, I'd like some better frobs to do multiple-file commits > without bringing up a vc-dir... What would you like to do? ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-23 7:17 ` Stefan Monnier 2010-06-23 7:45 ` David Kastrup 2010-06-23 9:00 ` Miles Bader @ 2010-06-23 18:45 ` Dan Nicolaescu 2010-06-23 22:04 ` Stefan Monnier 2 siblings, 1 reply; 38+ messages in thread From: Dan Nicolaescu @ 2010-06-23 18:45 UTC (permalink / raw) To: Stefan Monnier; +Cc: Juri Linkov, Štěpán Němec, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >>>> We could have a log-edit-extra-flags function that computes a set of >>>> extra flags, and pass those flags to vc-git-checkin. >>> If amend is triggered from a new command (like vc-amend), then there's no >>> need for any special support in log-edit. We could easily store the >>> relevant info in log-edit-callback or in vc-specific >>> buffer-local variables. >> The problem with that is that vc-amend does not integrate very well >> with C-x v v > > That doesn't bother me too much. I wish we could move further away from > C-x v v since I find it doesn't really work for modern VCSes. Not sure why you think so, but please don't try to take it away, it has been in use for so long, and it's still very useful. >> and it is very Git specific, > > Not sure about the "very", but in any case I don't see why that would > argue for integration inside "commit" rather than for the addition of > a separate command. You can do C-x v v to commit one file, then C-x v v and switch ammend on to add another file to the same commit. Also --amend it's still a property of commit, it's not a different command, why make it a different command in emacs? >> and given the reluctance of some other VCs to change history, it does >> not seem that it will be generalized. > > DaRCS already supports a more general form of Git's amend. Bazaar is No idea, we don't support DaRCS at all... > unlikely to go there, tho, indeed (although they are not opposed to > adding a more limited command to modify commit-comments). Don't know > about others. > >> More, it does not solve the problem of --signoff (which might be >> adopted by other VCs). > > What's the problem with sign-off? It seems this one fits perfectly well > inside a "Signed-Off-By:" header. Signed-Off-By: as an empty header does not work. --signoff does not have any arguments. >> Here's a solution that takes care of all problems and it is extensible. >> I adds one line to log-edit.el and two to vc.el. >> vc-git-log-edit-get-extra-flags-function does the work in vc-git.el > > Why add a var "log-edit-get-extra-flags-function" that's never used by > log-edit? It's used by vc-checkin, the main user of log-edit and by the modes derived from log-edit. Do you have a better proposal? > BTW, I think you can get what you want already without touching any of > vc.el and log-edit.el by making vc-git-log-edit-toggle-amend tweak the > log-edit-callback variable. That sounds fragile and much harder to understand for someone trying to develop vc.el 5 years from now ... > Maybe adding an `extra-flags' arg to `checkin' is the better approach, > but adding it everywhere just for the sake of Git doesn't sound too > good, which is why I'm resisting it. Why do you think that adding a top level vc-amend command that will only be used by git better than adding an unused argument (for now, it can become useful in the future) to other backends? > Also as a user I'd *really* like to have a clear visual feedback about > the fact that I'm amending something, which is another reason why having > it inside the headers is an attractive direction. For that in my local tree I have a minor mode instead of vc-git-log-edit-toggle-amend, it shows "amend" in the modeline. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-23 18:45 ` Dan Nicolaescu @ 2010-06-23 22:04 ` Stefan Monnier 2010-06-23 23:23 ` Dan Nicolaescu 0 siblings, 1 reply; 38+ messages in thread From: Stefan Monnier @ 2010-06-23 22:04 UTC (permalink / raw) To: Dan Nicolaescu; +Cc: Juri Linkov, Štěpán Němec, emacs-devel >> That doesn't bother me too much. I wish we could move further away from >> C-x v v since I find it doesn't really work for modern VCSes. > Not sure why you think so, but please don't try to take it away, it > has been in use for so long, and it's still very useful. I guess "C-x v v" in itself is not a bad key and it does something useful, so in this sense I don't intend to get rid of it, but its binding to vc-next-action is not very useful, since in modern VCSes, the only thing it can ever do (or close enough) is "commit", so the name is misleading. > Also --amend it's still a property of commit, it's not a different > command, why make it a different command in emacs? Yes, that's the main argument in favor of keeping it within the "vc-commit" command (currently misnamed vc-next-action). >>> and given the reluctance of some other VCs to change history, it does >>> not seem that it will be generalized. >> DaRCS already supports a more general form of Git's amend. Bazaar is > No idea, we don't support DaRCS at all... We do want to support DaRCS, tho. >>> More, it does not solve the problem of --signoff (which might be >>> adopted by other VCs). >> What's the problem with sign-off? It seems this one fits perfectly well >> inside a "Signed-Off-By:" header. > Signed-Off-By: as an empty header does not work. --signoff does not > have any arguments. Oh, that's why you tend to treat it like --amend. I see it's a problem, indeed, but I don't think that storing it into a buffer-local variable is a good answer. I'd rather have a "Sign-Off: yes" (you can still have a vc-git-log-edit-toggle-signoff command that adds/removes such a header). I was thinking of (re)using the Signed-Off-By thingy used by Linux kernel developers, but I'm not exactly sure of how it's intended to be used, so I'm not completely clear if and how it could be cleanly integrated with VC's need to support --signoff. >> Why add a var "log-edit-get-extra-flags-function" that's never used by >> log-edit? > It's used by vc-checkin, the main user of log-edit and by the modes > derived from log-edit. Do you have a better proposal? Yes: use a name that starts with "vc-" since it's a variable added for VC, and used exclusively by VC. >> Also as a user I'd *really* like to have a clear visual feedback about >> the fact that I'm amending something, which is another reason why having >> it inside the headers is an attractive direction. > For that in my local tree I have a minor mode instead of > vc-git-log-edit-toggle-amend, it shows "amend" in the modeline. Why not show it inside the buffer (e.g. in the header ;-) instead? Stefan ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-23 22:04 ` Stefan Monnier @ 2010-06-23 23:23 ` Dan Nicolaescu 2010-06-24 21:03 ` Stefan Monnier 0 siblings, 1 reply; 38+ messages in thread From: Dan Nicolaescu @ 2010-06-23 23:23 UTC (permalink / raw) To: Stefan Monnier; +Cc: Juri Linkov, Štěpán Němec, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> Also --amend it's still a property of commit, it's not a different >> command, why make it a different command in emacs? > > Yes, that's the main argument in favor of keeping it within the > "vc-commit" command (currently misnamed vc-next-action). > >>>> and given the reluctance of some other VCs to change history, it does >>>> not seem that it will be generalized. >>> DaRCS already supports a more general form of Git's amend. Bazaar is >> No idea, we don't support DaRCS at all... > > We do want to support DaRCS, tho. > >>>> More, it does not solve the problem of --signoff (which might be >>>> adopted by other VCs). >>> What's the problem with sign-off? It seems this one fits perfectly well >>> inside a "Signed-Off-By:" header. >> Signed-Off-By: as an empty header does not work. --signoff does not >> have any arguments. > > Oh, that's why you tend to treat it like --amend. I see it's a problem, > indeed, but I don't think that storing it into a buffer-local variable > is a good answer. I'd rather have a "Sign-Off: yes" (you can still But that means that Sign-off: no | off will still add --signoff. Which is confusing and wrong. More Sign-off: foo@bar.com gives the impression that it will use foo@bar.com to sign off, but that's not the case. --signoff does not have an argument, adding one artificially just creates unneeded confusion. > I was thinking of (re)using the Signed-Off-By thingy used by Linux kernel > developers, but I'm not exactly sure of how it's intended to be used, so > I'm not completely clear if and how it could be cleanly integrated with > VC's need to support --signoff. I've already presented a solution that works, and it does not create any confusion. >>> Why add a var "log-edit-get-extra-flags-function" that's never used by >>> log-edit? >> It's used by vc-checkin, the main user of log-edit and by the modes >> derived from log-edit. Do you have a better proposal? > > Yes: use a name that starts with "vc-" since it's a variable added for > VC, and used exclusively by VC. OK. >>> Also as a user I'd *really* like to have a clear visual feedback about >>> the fact that I'm amending something, which is another reason why having >>> it inside the headers is an attractive direction. >> For that in my local tree I have a minor mode instead of >> vc-git-log-edit-toggle-amend, it shows "amend" in the modeline. > > Why not show it inside the buffer (e.g. in the header ;-) instead? Because we want to insert the previous log when using --amend, so it's better to use a command instead of a header. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-23 23:23 ` Dan Nicolaescu @ 2010-06-24 21:03 ` Stefan Monnier 2010-06-24 21:18 ` Dan Nicolaescu 0 siblings, 1 reply; 38+ messages in thread From: Stefan Monnier @ 2010-06-24 21:03 UTC (permalink / raw) To: Dan Nicolaescu; +Cc: Juri Linkov, Štěpán Němec, emacs-devel >> Oh, that's why you tend to treat it like --amend. I see it's a problem, >> indeed, but I don't think that storing it into a buffer-local variable >> is a good answer. I'd rather have a "Sign-Off: yes" (you can still > But that means that > Sign-off: no | off > will still add --signoff. Not at all. The code gets to decide which values mean "add a --signoff argument". I'd start with only accepting "yes". > Which is confusing and wrong. It would be indeed, but that's not what I'm suggesting we do. > More > Sign-off: foo@bar.com > gives the impression that it will use foo@bar.com to sign off, but that's not the case. For the values other than "yes" we can highlight them with font-lock-warning-face and the backend can prompt the user when faced with such unclear values. We should also do something similar for unknown values of the "Fixed:" field, BTW. >> Why not show it inside the buffer (e.g. in the header ;-) instead? > Because we want to insert the previous log when using --amend, so it's > better to use a command instead of a header. The question is not "a command vs a header" but "a variable vs a header". In both cases we will want to provide a command (which will fetch the previous log, etc...). Stefan ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-24 21:03 ` Stefan Monnier @ 2010-06-24 21:18 ` Dan Nicolaescu 2010-06-24 22:25 ` Stefan Monnier 0 siblings, 1 reply; 38+ messages in thread From: Dan Nicolaescu @ 2010-06-24 21:18 UTC (permalink / raw) To: Stefan Monnier; +Cc: Juri Linkov, Štěpán Němec, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >>> Oh, that's why you tend to treat it like --amend. I see it's a problem, >>> indeed, but I don't think that storing it into a buffer-local variable >>> is a good answer. I'd rather have a "Sign-Off: yes" (you can still > >> But that means that >> Sign-off: no | off >> will still add --signoff. > > Not at all. The code gets to decide which values mean "add a --signoff > argument". I'd start with only accepting "yes". > >> Which is confusing and wrong. > > It would be indeed, but that's not what I'm suggesting we do. > >> More >> Sign-off: foo@bar.com >> gives the impression that it will use foo@bar.com to sign off, but that's not the case. > > For the values other than "yes" we can highlight them with > font-lock-warning-face and the backend can prompt the user when faced > with such unclear values. We should also do something similar for > unknown values of the "Fixed:" field, BTW. So now let's go back to the origin of this: there's a lot of code that needs to be added just to deal with the various possibilities for just the Signoff header, and even more for Ammend. Can you please show how this is better than adding a single argument to the vc-checkin method? (For which the code already exists). >>> Why not show it inside the buffer (e.g. in the header ;-) instead? >> Because we want to insert the previous log when using --amend, so it's >> better to use a command instead of a header. > > The question is not "a command vs a header" but "a variable vs > a header". In both cases we will want to provide a command (which will > fetch the previous log, etc...). So what happens if one deletes the Ammend: header? Accidentally or not? --amend and --signoff simply do not fit the header paradigm. Can we please admit that and move on? ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-24 21:18 ` Dan Nicolaescu @ 2010-06-24 22:25 ` Stefan Monnier 2010-06-24 23:14 ` Dan Nicolaescu 0 siblings, 1 reply; 38+ messages in thread From: Stefan Monnier @ 2010-06-24 22:25 UTC (permalink / raw) To: Dan Nicolaescu; +Cc: Juri Linkov, Štěpán Němec, emacs-devel > Can you please show how this is better than adding a single argument > to the vc-checkin method? (For which the code already exists). It makes more of the state plainly visible to the user, editable with Emacs's usual commands, rather than hidden in variables that are much more difficult for the user to control. I.e. it's more Emacsy by keeping things shallow. >>>> Why not show it inside the buffer (e.g. in the header ;-) instead? >>> Because we want to insert the previous log when using --amend, so it's >>> better to use a command instead of a header. >> The question is not "a command vs a header" but "a variable vs >> a header". In both cases we will want to provide a command (which will >> fetch the previous log, etc...). > So what happens if one deletes the Ammend: header? Accidentally or not? It means the commit doesn't amend. Same thing if you set your vars accidentally or if you call the toggle command accidentally, ... I really don't see it as a problem. Even if it's done accidentally, it's obvious for the user what the behavior will be, since it's written in plain text. > --amend and --signoff simply do not fit the header paradigm. > Can we please admit that and move on? I really don't see it. Stefan ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-24 22:25 ` Stefan Monnier @ 2010-06-24 23:14 ` Dan Nicolaescu 2010-06-25 1:16 ` Stefan Monnier 0 siblings, 1 reply; 38+ messages in thread From: Dan Nicolaescu @ 2010-06-24 23:14 UTC (permalink / raw) To: Stefan Monnier; +Cc: Juri Linkov, Štěpán Němec, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> Can you please show how this is better than adding a single argument >> to the vc-checkin method? (For which the code already exists). > > It makes more of the state plainly visible to the user, editable with > Emacs's usual commands, rather than hidden in variables that are much > more difficult for the user to control. How is it more difficult to control something that has an explicit command attached to it? (that you've stated that we need to have anyway) > I.e. it's more Emacsy by keeping things shallow. That's not quite true for a lot of things we do in Emacs. >>>>> Why not show it inside the buffer (e.g. in the header ;-) instead? >>>> Because we want to insert the previous log when using --amend, so it's >>>> better to use a command instead of a header. >>> The question is not "a command vs a header" but "a variable vs >>> a header". In both cases we will want to provide a command (which will >>> fetch the previous log, etc...). >> So what happens if one deletes the Ammend: header? Accidentally or not? > > It means the commit doesn't amend. Same thing if you set your vars > accidentally or if you call the toggle command accidentally, ... > > I really don't see it as a problem. Even if it's done accidentally, > it's obvious for the user what the behavior will be, since it's written > in plain text. Why deal with any potential complications that are not needed at all? >> --amend and --signoff simply do not fit the header paradigm. >> Can we please admit that and move on? > > I really don't see it. Are willing to implement your version and ask users if they prefer it? I've implemented my version and have been using it for > 6 months, it's much better to not have to deal with headers. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-24 23:14 ` Dan Nicolaescu @ 2010-06-25 1:16 ` Stefan Monnier 2010-06-25 2:27 ` Dan Nicolaescu 0 siblings, 1 reply; 38+ messages in thread From: Stefan Monnier @ 2010-06-25 1:16 UTC (permalink / raw) To: Dan Nicolaescu; +Cc: Juri Linkov, Štěpán Němec, emacs-devel >>> Can you please show how this is better than adding a single argument >>> to the vc-checkin method? (For which the code already exists). >> It makes more of the state plainly visible to the user, editable with >> Emacs's usual commands, rather than hidden in variables that are much >> more difficult for the user to control. > How is it more difficult to control something that has an explicit > command attached to it? (that you've stated that we need to have > anyway) It's more difficult because you're limited to what the command lets you do, whereas with plain text, you can do anything you like (including running the provided command). >> I.e. it's more Emacsy by keeping things shallow. > That's not quite true for a lot of things we do in Emacs. And in most cases, that is a problem. Sometimes it's justified, and sometimes it's just because noone has gone through the trouble to make it better. >> It means the commit doesn't amend. Same thing if you set your vars >> accidentally or if you call the toggle command accidentally, ... >> >> I really don't see it as a problem. Even if it's done accidentally, >> it's obvious for the user what the behavior will be, since it's written >> in plain text. > Why deal with any potential complications that are not needed at all? What complications are you referring to? >>> --amend and --signoff simply do not fit the header paradigm. >>> Can we please admit that and move on? >> I really don't see it. > Are willing to implement your version and ask users if they prefer it? Don't know. But I don't want the implementation you propose. I want the information to be shallow and readily available for the user to manipulate as she sees fit. Stefan ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-25 1:16 ` Stefan Monnier @ 2010-06-25 2:27 ` Dan Nicolaescu 2010-06-25 11:44 ` Miles Bader 0 siblings, 1 reply; 38+ messages in thread From: Dan Nicolaescu @ 2010-06-25 2:27 UTC (permalink / raw) To: Stefan Monnier; +Cc: Juri Linkov, Štěpán Němec, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >>>> Can you please show how this is better than adding a single argument >>>> to the vc-checkin method? (For which the code already exists). >>> It makes more of the state plainly visible to the user, editable with >>> Emacs's usual commands, rather than hidden in variables that are much >>> more difficult for the user to control. >> How is it more difficult to control something that has an explicit >> command attached to it? (that you've stated that we need to have >> anyway) > > It's more difficult because you're limited to what the command lets > you do, whereas with plain text, you can do anything you like > (including running the provided command). As a general principle this sounds fine, but in this particular case it does not hold too much water. Your general case includes the particular case, but it adds unneeded convulutions due to having to deal with random editing. Also, a lot of times specific solutions are much better than generic ones. BTW, what you propose is the perforce model, you might want to talk to some perforce users about how good it is to have to edit for various commands instead of providing an imperative interface. >>> It means the commit doesn't amend. Same thing if you set your vars >>> accidentally or if you call the toggle command accidentally, ... >>> >>> I really don't see it as a problem. Even if it's done accidentally, >>> it's obvious for the user what the behavior will be, since it's written >>> in plain text. > >> Why deal with any potential complications that are not needed at all? > > What complications are you referring to? Having to educate the users what they can and cannot do with the markup and the code to deal with it that has ZERO end-user value. >>>> --amend and --signoff simply do not fit the header paradigm. >>>> Can we please admit that and move on? >>> I really don't see it. >> Are willing to implement your version and ask users if they prefer it? > > Don't know. But I don't want the implementation you propose. I want > the information to be shallow and readily available for the user to > manipulate as she sees fit. It seems that this is your personal preference and it's not actually technically justified. Given that you don't really plan to do anything about it, and I am convinced that it's the wrong solution for the problem based on my personal experience both with this particular feature and from using perforce. How about you yield here to the person that has done the vast majority of work in the VC area in the last few years, both in terms of new features and in terms of bug fixing? If users come back and ask for markup for amend and commit we can add them later, but at this particular point this is just delaying giving end users access to some very useful features. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-25 2:27 ` Dan Nicolaescu @ 2010-06-25 11:44 ` Miles Bader 2010-06-26 5:09 ` Dan Nicolaescu 2010-06-26 10:11 ` David Kastrup 0 siblings, 2 replies; 38+ messages in thread From: Miles Bader @ 2010-06-25 11:44 UTC (permalink / raw) To: Dan Nicolaescu Cc: Juri Linkov, Štěpán Němec, Stefan Monnier, emacs-devel Dan Nicolaescu <dann@gnu.org> writes: > Given that you don't really plan to do anything about it, and I am > convinced that it's the wrong solution for the problem based on my > personal experience both with this particular feature and from using > perforce. I kinda like Stefan's method better too. It ("Stefan's method") seems much more flexible, albeit maybe a little more open to risk from the user fiddles where he shouldn't have. I also agree with Stefan that it's somewhat more "emacsy", as it leverages ordinary text in a buffer instead of hidden magic. Like other such cases I think it will thus tend to appeal more to experienced users and maybe scare off noobs a bit; but since I am an experienced user, I'd prefer flexibility and transparency. As for the noobs, I think they win in the long run, even if they're a bit put off in the short run, because the relative transparency of such a mechanism makes it easier for them to do more advanced things without getting tripped up by some hidden behind-the-scenes mechanism. A dedicated "vc-append" (or whatever) command which sets up the log buffer appropriately with a pre-stuffed message and appropriate meta-data seems a reasonable thing to me, but I really like the use of the log buffer itself to hold all meta-data. -Miles p.s. Actually I don't know if it was Stefan that actually came up with that method originally, but anyway, you get the idea....... -- Suburbia: where they tear out the trees and then name streets after them. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-25 11:44 ` Miles Bader @ 2010-06-26 5:09 ` Dan Nicolaescu 2010-07-01 0:01 ` Stefan Monnier 2010-06-26 10:11 ` David Kastrup 1 sibling, 1 reply; 38+ messages in thread From: Dan Nicolaescu @ 2010-06-26 5:09 UTC (permalink / raw) To: Miles Bader Cc: Juri Linkov, Štěpán Němec, Stefan Monnier, emacs-devel Miles Bader <miles@gnu.org> writes: > Dan Nicolaescu <dann@gnu.org> writes: >> Given that you don't really plan to do anything about it, and I am >> convinced that it's the wrong solution for the problem based on my >> personal experience both with this particular feature and from using >> perforce. > > I kinda like Stefan's method better too. > > It ("Stefan's method") seems much more flexible, albeit maybe a little > more open to risk from the user fiddles where he shouldn't have. > > I also agree with Stefan that it's somewhat more "emacsy", as it > leverages ordinary text in a buffer instead of hidden magic. Like other > such cases I think it will thus tend to appeal more to experienced users > and maybe scare off noobs a bit; but since I am an experienced user, I'd > prefer flexibility and transparency. As for the noobs, I think they win > in the long run, even if they're a bit put off in the short run, because > the relative transparency of such a mechanism makes it easier for them > to do more advanced things without getting tripped up by some hidden > behind-the-scenes mechanism. > > A dedicated "vc-append" (or whatever) command which sets up the log > buffer appropriately with a pre-stuffed message and appropriate > meta-data seems a reasonable thing to me, but I really like the use of > the log buffer itself to hold all meta-data. Are you interested in implementing that version and asking users which one they prefer? My version exists, it's functional, it's probably less than 25 lines of code (hard to count exactly, I have many other indepenedent changes). This discussion does not seem to go anywhere, all the arguments have been made already. And it's quite a trivial issue to start with... So I'd like to request a resolution. I have provided a working implementation and motivation why it's the right thing to do, and why the other proposal is not a good idea. It seems that there are 3 possible courses of action. 1. check in my code 2. reject my code and do nothing 3. someone else volunteers to provide an alternative and check that in [To be fair, it would be nice if 3 was chosen to ask at least a few users if the prefer 1 or 3] ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-26 5:09 ` Dan Nicolaescu @ 2010-07-01 0:01 ` Stefan Monnier 0 siblings, 0 replies; 38+ messages in thread From: Stefan Monnier @ 2010-07-01 0:01 UTC (permalink / raw) To: Dan Nicolaescu Cc: Juri Linkov, emacs-devel, Štěpán Němec, Miles Bader > It seems that there are 3 possible courses of action. > 1. check in my code I already rejected this option. > 2. reject my code and do nothing > 3. someone else volunteers to provide an alternative and check that in As you know, I already re-implemented your original "commit args for authors and subject lines", so who know, I may re-implement your amend patch if noone else gets there before I do. Stefan "travelling" ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-25 11:44 ` Miles Bader 2010-06-26 5:09 ` Dan Nicolaescu @ 2010-06-26 10:11 ` David Kastrup 2010-06-28 21:04 ` Juri Linkov 1 sibling, 1 reply; 38+ messages in thread From: David Kastrup @ 2010-06-26 10:11 UTC (permalink / raw) To: emacs-devel Miles Bader <miles@gnu.org> writes: > Dan Nicolaescu <dann@gnu.org> writes: >> Given that you don't really plan to do anything about it, and I am >> convinced that it's the wrong solution for the problem based on my >> personal experience both with this particular feature and from using >> perforce. > > I kinda like Stefan's method better too. It seems like it would be quite more problematic to script/macro/batch. Perhaps one needs to think about an argument for non-interactive use that makes it easy to substitute for the editing of the commit message? So if one pre-specifies command line options in some manner, the respective pseudo-lines in the commit message are not generated in the first place? That way, one can use Stefan's method by default, but rather transparently move to as much of Dan's method as one likes. Anyway, the principal problem appears similar to web forums as opposed to Usenet. They seem user-friendly on the surface, but I can't keep up with more than a few web forums because of time restraints. Handling dozens of Usenet groups via gnus, in contrast, is easy. -- David Kastrup ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-26 10:11 ` David Kastrup @ 2010-06-28 21:04 ` Juri Linkov 0 siblings, 0 replies; 38+ messages in thread From: Juri Linkov @ 2010-06-28 21:04 UTC (permalink / raw) To: David Kastrup; +Cc: emacs-devel > That way, one can use Stefan's method by default, but rather > transparently move to as much of Dan's method as one likes. I wonder if, for instance, I'd like to bind `C-c C-c' to a command that asks for confirmation of command line arguments in the minibuffer, would it be possible to implement such a command with Stefan's method or Dan's method? -- Juri Linkov http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-11 14:18 ` Stefan Monnier 2010-06-11 16:14 ` Štěpán Němec @ 2010-06-11 17:34 ` Dan Nicolaescu 2010-06-11 19:27 ` Juri Linkov 2 siblings, 0 replies; 38+ messages in thread From: Dan Nicolaescu @ 2010-06-11 17:34 UTC (permalink / raw) To: Stefan Monnier; +Cc: Juri Linkov, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> For --amend I think we should have a different mechanism that does not >> involve editing the text in the *VC-log* buffer, it's a more >> destructive operation and it's not a good idea to allow the user to do >> it by mistake by just placing an Amend: line in the buffer. > > You might be right, but I don't think it would be terribly bad to have it > be editable in the buffer (after all it's also just an "--amend" away when > committing on the command line). > > I'm not sure what UI you're considering to use and what users would > want, because I've never used Git's amend feature. But I think it > should be considered in the light of similar functionality in other VCS, > such as maybe "cvs admin -m" and DaRCS's "amend-record". > > For PCL-CVS what I offered was a way to edit a log-message (typically > called from the log-view buffer). But admittedly, "cvs admin -m" is > a fairly different beast from Git's amend. We already have a UI for that in VC: from the log buffer you can use "e" to edit the commit log. git commit --amend is different, it can be used to add more files to the same commit, and it can only affect the last commit. > If we only consider Git's and DaRCS's forms of amend, I'd say that the > "Amend:" header might be a good approach, and that it should specify the > revision/patch that's amended. So for Git, you could have a command > that inserts "Amend: <SHA-1>" and then the backend could check that the > SHA-1 is the right one (which would avoid accidental use). That looks like a bad UI: the user sees information that is unnecessary, and might feel compelled to verify if the sha1 is indeed to correct one (i.e. time wasted for no good reason). Other VCs have commands that we might want to support in a similar manner, for example: hg has --close-branch So besides the markup in the VC-log buffer, we need another way to pass arguments to the commit command. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-11 14:18 ` Stefan Monnier 2010-06-11 16:14 ` Štěpán Němec 2010-06-11 17:34 ` Dan Nicolaescu @ 2010-06-11 19:27 ` Juri Linkov 2010-06-11 20:16 ` Dan Nicolaescu 2010-06-11 20:35 ` Stefan Monnier 2 siblings, 2 replies; 38+ messages in thread From: Juri Linkov @ 2010-06-11 19:27 UTC (permalink / raw) To: Stefan Monnier; +Cc: Dan Nicolaescu, emacs-devel > If we only consider Git's and DaRCS's forms of amend, I'd say that the > "Amend:" header might be a good approach, and that it should specify the > revision/patch that's amended. So for Git, you could have a command > that inserts "Amend: <SHA-1>" and then the backend could check that the > SHA-1 is the right one (which would avoid accidental use). A more general variant would be a header that allows to specify command line arguments like Arguments: --amend --signoff This will provide interchangeability: the user will be able to copy arguments from the *VC-log* buffer to the external command line and back to the *VC-log* buffer to construct the necessary command line. -- Juri Linkov http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-11 19:27 ` Juri Linkov @ 2010-06-11 20:16 ` Dan Nicolaescu 2010-06-11 20:38 ` Juri Linkov ` (2 more replies) 2010-06-11 20:35 ` Stefan Monnier 1 sibling, 3 replies; 38+ messages in thread From: Dan Nicolaescu @ 2010-06-11 20:16 UTC (permalink / raw) To: Juri Linkov; +Cc: Stefan Monnier, emacs-devel Juri Linkov <juri@jurta.org> writes: >> If we only consider Git's and DaRCS's forms of amend, I'd say that the >> "Amend:" header might be a good approach, and that it should specify the >> revision/patch that's amended. So for Git, you could have a command >> that inserts "Amend: <SHA-1>" and then the backend could check that the >> SHA-1 is the right one (which would avoid accidental use). > > A more general variant would be a header that allows to specify > command line arguments like > > Arguments: --amend --signoff > > This will provide interchangeability: the user will be able to copy > arguments from the *VC-log* buffer to the external command line and > back to the *VC-log* buffer to construct the necessary command line. That's very ugly from the UI point of view, it's not better than doing the same thing from the command line directly. More, for --amend it's desirable to copy the contents of the original log in the *VC-log* buffer, so that the user can edit it, and check in the modified version. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-11 20:16 ` Dan Nicolaescu @ 2010-06-11 20:38 ` Juri Linkov 2010-06-11 23:48 ` W Dan Meyer 2010-06-12 2:21 ` Dan Nicolaescu 2010-06-11 23:44 ` Thien-Thi Nguyen 2010-06-12 20:15 ` Stefan Monnier 2 siblings, 2 replies; 38+ messages in thread From: Juri Linkov @ 2010-06-11 20:38 UTC (permalink / raw) To: Dan Nicolaescu; +Cc: Stefan Monnier, emacs-devel >> Arguments: --amend --signoff >> >> This will provide interchangeability: the user will be able to copy >> arguments from the *VC-log* buffer to the external command line and >> back to the *VC-log* buffer to construct the necessary command line. > > That's very ugly from the UI point of view, it's not better than doing > the same thing from the command line directly. In Emacs it's easier to select files and to write a log message than doing the same in the command line. But currently vc mode is limited to a small set of supported arguments. Allowing to specify the command line arguments will give the user the full power of the command line when doing vc operations from Emacs. > More, for --amend it's desirable to copy the contents of the original > log in the *VC-log* buffer, so that the user can edit it, and check in > the modified version. Maybe, --amend is a special case that requires special processing. But generally it's also desirable to be able to specify the command line arguments from the *VC-log* buffer. -- Juri Linkov http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-11 20:38 ` Juri Linkov @ 2010-06-11 23:48 ` W Dan Meyer 2010-06-12 20:23 ` Juri Linkov 2010-06-12 2:21 ` Dan Nicolaescu 1 sibling, 1 reply; 38+ messages in thread From: W Dan Meyer @ 2010-06-11 23:48 UTC (permalink / raw) To: Juri Linkov; +Cc: Dan Nicolaescu, Stefan Monnier, emacs-devel Juri Linkov <juri@jurta.org> writes: >>> Arguments: --amend --signoff >>> >>> This will provide interchangeability: the user will be able to copy >>> arguments from the *VC-log* buffer to the external command line and >>> back to the *VC-log* buffer to construct the necessary command line. >> >> That's very ugly from the UI point of view, it's not better than doing >> the same thing from the command line directly. > > In Emacs it's easier to select files and to write a log message > than doing the same in the command line. > > But currently vc mode is limited to a small set of supported arguments. > Allowing to specify the command line arguments will give the user the > full power of the command line when doing vc operations from Emacs. Before I started using VC for real, I used to issue (and I still do it sometimes), M-&, and just type the vcs command - this way I've had a some completion, asynchronous command and output in a separate buffer. Maybe something like C-x g . would be good but: - with completion of possible commands (the interface is not really different, it is always a driver name and then token representing command) - adjusting the output (choosing output buffer mode, being quiet, etc.) - being quiet and synchronous for some class of commands This would be based on the assumption that almost every time. `log' command produces log, diff produces `diff' and `rm' could be silent and not show the buffer (plus it could be possibly synchronous). So adjusting those two things (completion and processing output) would give certain advantages but allowed user to type any flavour of command he wished. BTW: There could be in Emacs sort of way of specifying grammar for parsing command-line commands apart from usual user interfaces. It could be generated from <command> -h. That could allow completion with online help, and if the format is -h standarised enough even completion of certain types of entries. (Like <file>). > >> More, for --amend it's desirable to copy the contents of the original >> log in the *VC-log* buffer, so that the user can edit it, and check in >> the modified version. > > Maybe, --amend is a special case that requires special processing. > But generally it's also desirable to be able to specify the command line > arguments from the *VC-log* buffer. Wojciech ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-11 23:48 ` W Dan Meyer @ 2010-06-12 20:23 ` Juri Linkov 0 siblings, 0 replies; 38+ messages in thread From: Juri Linkov @ 2010-06-12 20:23 UTC (permalink / raw) To: W Dan Meyer; +Cc: Dan Nicolaescu, Stefan Monnier, emacs-devel > BTW: There could be in Emacs sort of way of specifying grammar for > parsing command-line commands apart from usual user interfaces. It could > be generated from <command> -h. That could allow completion with online > help, and if the format is -h standarised enough even completion of > certain types of entries. (Like <file>). Isn't it what pcomplete.el, pcmpl-cvs.el and other pcmpl-*.el try to do? -- Juri Linkov http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-11 20:38 ` Juri Linkov 2010-06-11 23:48 ` W Dan Meyer @ 2010-06-12 2:21 ` Dan Nicolaescu 1 sibling, 0 replies; 38+ messages in thread From: Dan Nicolaescu @ 2010-06-12 2:21 UTC (permalink / raw) To: Juri Linkov; +Cc: Stefan Monnier, emacs-devel Juri Linkov <juri@jurta.org> writes: >>> Arguments: --amend --signoff >>> >>> This will provide interchangeability: the user will be able to copy >>> arguments from the *VC-log* buffer to the external command line and >>> back to the *VC-log* buffer to construct the necessary command line. >> >> That's very ugly from the UI point of view, it's not better than doing >> the same thing from the command line directly. > > In Emacs it's easier to select files and to write a log message > than doing the same in the command line. > > But currently vc mode is limited to a small set of supported arguments. > Allowing to specify the command line arguments will give the user the > full power of the command line when doing vc operations from Emacs. > >> More, for --amend it's desirable to copy the contents of the original >> log in the *VC-log* buffer, so that the user can edit it, and check in >> the modified version. > > Maybe, --amend is a special case that requires special processing. > But generally it's also desirable to be able to specify the command line > arguments from the *VC-log* buffer. I'd like to support the special case well, it's a more frequent operation. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-11 20:16 ` Dan Nicolaescu 2010-06-11 20:38 ` Juri Linkov @ 2010-06-11 23:44 ` Thien-Thi Nguyen 2010-06-12 20:15 ` Stefan Monnier 2 siblings, 0 replies; 38+ messages in thread From: Thien-Thi Nguyen @ 2010-06-11 23:44 UTC (permalink / raw) To: emacs-devel () Dan Nicolaescu <dann@gnu.org> () Fri, 11 Jun 2010 16:16:18 -0400 Juri Linkov <juri@jurta.org> writes: > This will provide interchangeability: the user will be able to copy > arguments from the *VC-log* buffer to the external command line and > back to the *VC-log* buffer to construct the necessary command line. That's very ugly from the UI point of view, it's not better than doing the same thing from the command line directly. I disagree on the aesthetics. Straightforward editable text is what Emacs is good for. Such an interface invites user-defined UI. For example, keyboard macros are compatible with it, as are dedicated commands (what you seem to be pushing), as are advised functions, etc. It is also extensible in the other direction (for people who have extended, or ~/bin-wrapped programs). Perlis sez: It is better to have 100 functions operate on one data structure than 10 functions on 10 data structures. thi ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-11 20:16 ` Dan Nicolaescu 2010-06-11 20:38 ` Juri Linkov 2010-06-11 23:44 ` Thien-Thi Nguyen @ 2010-06-12 20:15 ` Stefan Monnier 2 siblings, 0 replies; 38+ messages in thread From: Stefan Monnier @ 2010-06-12 20:15 UTC (permalink / raw) To: Dan Nicolaescu; +Cc: Juri Linkov, emacs-devel > More, for --amend it's desirable to copy the contents of the original > log in the *VC-log* buffer, so that the user can edit it, and check in > the modified version. I'd tend to agree. In this case, we'd probably want to provide this via a separate command (we could call it vc-amend). Stefan ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: support for git commit --amend/--signoff 2010-06-11 19:27 ` Juri Linkov 2010-06-11 20:16 ` Dan Nicolaescu @ 2010-06-11 20:35 ` Stefan Monnier 1 sibling, 0 replies; 38+ messages in thread From: Stefan Monnier @ 2010-06-11 20:35 UTC (permalink / raw) To: Juri Linkov; +Cc: Dan Nicolaescu, emacs-devel > A more general variant would be a header that allows to specify > command line arguments like > Arguments: --amend --signoff > This will provide interchangeability: the user will be able to copy > arguments from the *VC-log* buffer to the external command line and > back to the *VC-log* buffer to construct the necessary command line. I like that. I'm not sure it is the ultimate answer to the --amend functionality, but it's a good feature to have, so the user can always get what she wants by providing the args manually. Stefan ^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2010-07-01 0:01 UTC | newest] Thread overview: 38+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-06-11 6:19 support for git commit --amend/--signoff Dan Nicolaescu 2010-06-11 8:09 ` Juri Linkov 2010-06-11 13:23 ` Dan Nicolaescu 2010-06-11 14:18 ` Stefan Monnier 2010-06-11 16:14 ` Štěpán Němec 2010-06-11 20:26 ` Stefan Monnier 2010-06-12 2:19 ` Dan Nicolaescu 2010-06-12 19:59 ` Juri Linkov 2010-06-12 20:19 ` Stefan Monnier 2010-06-19 6:38 ` Dan Nicolaescu 2010-06-23 7:17 ` Stefan Monnier 2010-06-23 7:45 ` David Kastrup 2010-06-23 9:00 ` Miles Bader 2010-06-23 18:55 ` Dan Nicolaescu 2010-06-23 18:45 ` Dan Nicolaescu 2010-06-23 22:04 ` Stefan Monnier 2010-06-23 23:23 ` Dan Nicolaescu 2010-06-24 21:03 ` Stefan Monnier 2010-06-24 21:18 ` Dan Nicolaescu 2010-06-24 22:25 ` Stefan Monnier 2010-06-24 23:14 ` Dan Nicolaescu 2010-06-25 1:16 ` Stefan Monnier 2010-06-25 2:27 ` Dan Nicolaescu 2010-06-25 11:44 ` Miles Bader 2010-06-26 5:09 ` Dan Nicolaescu 2010-07-01 0:01 ` Stefan Monnier 2010-06-26 10:11 ` David Kastrup 2010-06-28 21:04 ` Juri Linkov 2010-06-11 17:34 ` Dan Nicolaescu 2010-06-11 19:27 ` Juri Linkov 2010-06-11 20:16 ` Dan Nicolaescu 2010-06-11 20:38 ` Juri Linkov 2010-06-11 23:48 ` W Dan Meyer 2010-06-12 20:23 ` Juri Linkov 2010-06-12 2:21 ` Dan Nicolaescu 2010-06-11 23:44 ` Thien-Thi Nguyen 2010-06-12 20:15 ` Stefan Monnier 2010-06-11 20:35 ` Stefan Monnier
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).