Skip to content

Loading…

magit-commit-for-autosquash: new command #585

Closed
wants to merge 1 commit into from

4 participants

@azuk

This command makes it possible to create fixup! commits for the autosquash feature of git-rebase. I've used it practically every day for quite some time now and hope it will make amending non-HEAD commits more convenient for others as well. It's of course beneficial to "pool" fixup commits but even if you make just one and apply it immediately, ' E C-x # is a rather quick way to do it (I have ideas to make that even easier but I think this is a reasonable first step).

I'll happily give my blessing for an improvement that also allows squash! commits to be created (with, for example, a prefix argument or so that ' ' creates a fixup! commit and ' s creates a squash! commit), but I've never needed those so whoever does gets to add that feature.

@sigma sigma commented on an outdated diff
magit.el
@@ -5556,6 +5557,23 @@ for the file whose log must be displayed."
;;; Miscellaneous
+(magit-define-command commit-for-autosquash ()
+ "Make a fixup! commit to be autosquashed to the commit at point.
+If there are staged changes, those are committed. If there are
+no staged changes, all modifications and deletions are
+committed (git commit --all). See git-rebase(1) for a
+description of the autosquash feature."
+ (interactive)
+ (when (and (not (string= "1" (magit-get "rebase.autosquash")))
@sigma Magit member
sigma added a note

should probably be (magit-get-boolean "rebase" "autosquash")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@sigma sigma commented on an outdated diff
magit.el
@@ -5556,6 +5557,23 @@ for the file whose log must be displayed."
;;; Miscellaneous
+(magit-define-command commit-for-autosquash ()
+ "Make a fixup! commit to be autosquashed to the commit at point.
+If there are staged changes, those are committed. If there are
+no staged changes, all modifications and deletions are
+committed (git commit --all). See git-rebase(1) for a
+description of the autosquash feature."
+ (interactive)
+ (when (and (not (string= "1" (magit-get "rebase.autosquash")))
+ (y-or-n-p "rebase.autosquash is not set. Do you want it to be set now? "))
+ (magit-set "rebase.autosquash" "1"))
+ (let ((commit-subject (magit-trim-line (magit-format-commit (magit-commit-at-point) "%s"))))
+ (apply #'magit-run-git-async "commit" "-m"
+ (if (string-match "^fixup! " commit-subject)
@sigma Magit member
sigma added a note

I think we should test for (fixup|squash)!, just in case some of those commits have been created outside of magit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@sigma sigma commented on an outdated diff
magit.el
((4 lines not shown))
+(magit-define-command commit-for-autosquash ()
+ "Make a fixup! commit to be autosquashed to the commit at point.
+If there are staged changes, those are committed. If there are
+no staged changes, all modifications and deletions are
+committed (git commit --all). See git-rebase(1) for a
+description of the autosquash feature."
+ (interactive)
+ (when (and (not (string= "1" (magit-get "rebase.autosquash")))
+ (y-or-n-p "rebase.autosquash is not set. Do you want it to be set now? "))
+ (magit-set "rebase.autosquash" "1"))
+ (let ((commit-subject (magit-trim-line (magit-format-commit (magit-commit-at-point) "%s"))))
+ (apply #'magit-run-git-async "commit" "-m"
+ (if (string-match "^fixup! " commit-subject)
+ commit-subject
+ (concat "fixup! " commit-subject))
+ (if (magit-anything-staged-p) '() '("--all")))))
@sigma Magit member
sigma added a note

that kind of logic should certainly be shared with the regular commit one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@azuk

Excellent observations. I hope I understood correctly that the last comment referred to what the regular commit does when nothing is staged. In addition to addressing your points, I fixed the parameters of magit-set.

For the purposes of this command, I didn't see any difference between magit-commit-all-when-nothing-staged being ask or ask-stage, so they are treated in the same way.

@azuk

How about this version with support also for squash! commits and use of magit-key-mode?

@sigma
Magit member

I don't think it's useful to provide 2 entry points. After all those commits are by definition transient, so I don't think anyone has a real preference regarding the actual prefix used. And even if so, a custom variable would be more than sufficient, there's no need for a dynamic way of choosing one or the other.

I'm more concerned about the fact that some of the commit logic (when to add the --all flag) is duplicated here. Looks like we're missing an abstraction somewhere.

Note that the config variable access is still not correct. It should be for example: (magit-get-boolean "rebase" "autosquash")

@azuk azuk magit-{fixup,squash}-for-autosquash: new commands
For creating commits for git-rebase's autosquash feature.
1310dab
@azuk

You seem to think that there is no functional difference between using fixup! and squash! prefixes. There is; squash! means squash command will be used and fixup! means fixup command will be used (and there is a difference between those in interactive rebase).

Sharing the --all condition feels like a rather trivial concern to me and tinkering with that is not something I want to use my time for, so if you feel that is needed, I'll drop this pull request.

I changed the use of config variables.

@afrepues

I just learned about autosquash through this, and having support for its use in magit would be very useful.

@sigma after reading the documentation for rebase --autosquash I agree with the original author that the two entry points are needed. I am with you regarding the duplication of commit logic in magit-commit-for-autosquash.

I don't like either how the on every usage of the feature the user is asked about activating rebase.autosquash, I'd find it disruptive of my workflow, and can affect the usage of tools others than magit. I'd rather:

  • add a magit boolean option to activate autosquash, and add the --autosquash in magit-interactive-rebase if the option is t, and maybe even make the commands not available or be a no-op as well if it is not t
  • if the commands are enabled/accesible, just emit a message if either the option is not set or config.autosquash is false
@sigma sigma was assigned
@tarsius
Magit member

I have looked at this with the intention to merge it after minor cleanup and finding a better way to enable rebase.autosquash. But after closer inspection I think we should not add new commands and instead extend magit-log-edit to support these use cases.

magit-log-edit already uses prefix commands for some "special condition" commits (empty commits and amending). This could extended so that using two prefix arguments causes the user to be asked whether we want to make an empty, fixup! or squash! commit.

But once we start thinking into that direction a lot of alternative ways come into sight and it becomes clear that there isn't any easy way to do this correctly.

We could e.g. convert magit-log-edit into a magit-key-mode command, but that would quickly become annoying when making "normal" commits. So instead only pop-up a key-mode buffer when magit-log-edit mode is invoked with a prefix argument? Or convert the commit into a fixup! commit once inside the log-edit buffer? For squash! that makes less sense though.

Thinking this through properly will take some time and discussion. It also touches on other issues I have come across. E.g. why does one need a prefix argument to select the remote to push to in magit-push; seems completely random as we are already using key-mode. Except that key-mode does not support doing it the way I imagine. Etc.

So I am afraid we cannot merge this right now. That doesn't mean I don't find this functionality useful, just that merging it now would likely mean it would get reimplemented in an incompatible way later on. So I suggest you turn this into an external package. I will certainly mirror it on the Emacsmirror (provided you use proper library headers) and will likely install it myself. But I don't want to merge this; and @sigma did not sound very exited either -- so I am closing this now.

@tarsius tarsius closed this
@tarsius
Magit member

#993 is a new pr that provides such functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 21, 2013
  1. @azuk

    magit-{fixup,squash}-for-autosquash: new commands

    azuk committed
    For creating commits for git-rebase's autosquash feature.
This page is out of date. Refresh to see the latest.
Showing with 57 additions and 1 deletion.
  1. +7 −1 magit-key-mode.el
  2. +38 −0 magit.el
  3. +12 −0 magit.texi
View
8 magit-key-mode.el
@@ -166,7 +166,13 @@
("r" "Reset" magit-bisect-reset)
("s" "Start" magit-bisect-start)
("u" "Run" magit-bisect-run)
- ("v" "Visualize" magit-bisect-visualize))))
+ ("v" "Visualize" magit-bisect-visualize)))
+
+ (autosquash
+ (man-page "git-rebase")
+ (actions
+ ("'" "Fixup!" magit-fixup-for-autosquash)
+ ("s" "Squash!" magit-squash-for-autosquash))))
"Holds the key, help, function mapping for the log-mode.
If you modify this make sure you reset `magit-key-mode-key-maps'
to nil.")
View
38 magit.el
@@ -762,6 +762,7 @@ This is calculated from `magit-highlight-indentation'.")
(define-key map (kbd "+") 'magit-diff-larger-hunks)
(define-key map (kbd "0") 'magit-diff-default-hunks)
(define-key map (kbd "h") 'magit-toggle-diff-refine-hunk)
+ (define-key map (kbd "'") 'magit-key-mode-popup-autosquash)
map))
(defvar magit-commit-mode-map
@@ -5639,6 +5640,43 @@ for the file whose log must be displayed."
;;; Miscellaneous
+(defun magit-commit-for-autosquash (prefix)
+ "Does the work of magit-fixup-for-autosquash and magit-squash-for-autosquash.
+`prefix' is either \"fixup!\" or \"squash!\""
+ (when (and (not (magit-get-boolean "rebase" "autosquash"))
+ (y-or-n-p "rebase.autosquash is not set. Do you want it to be set now? "))
+ (magit-set "1" "rebase" "autosquash"))
+ (let* ((commit-subject (magit-trim-line (magit-format-commit (magit-commit-at-point) "%s")))
+ (new-subject (concat prefix " "
+ (if (string-match "^\\(squash\\|fixup\\)! " commit-subject)
+ (substring commit-subject (match-end 0))
+ commit-subject))))
+ (apply #'magit-run-git-async "commit" "-m" new-subject
+ (cond ((magit-anything-staged-p) '())
+ ((or (eq magit-commit-all-when-nothing-staged t)
+ (and magit-commit-all-when-nothing-staged
+ (y-or-n-p "Nothing staged. Commit all unstaged changes? ")))
+ '("--all"))
+ (t (error "Nothing to commit."))))))
+
+(magit-define-command fixup-for-autosquash ()
+ "Make a fixup! commit to be autosquashed to the commit at point.
+If there are staged changes, those are committed. If there are
+no staged changes, `magit-commit-all-when-nothing-staged'
+determines what is done. See git-rebase(1) for a description of
+the autosquash feature."
+ (interactive)
+ (magit-commit-for-autosquash "fixup!"))
+
+(magit-define-command squash-for-autosquash ()
+ "Make a squash! commit to be autosquashed to the commit at point.
+If there are staged changes, those are committed. If there are
+no staged changes, `magit-commit-all-when-nothing-staged'
+determines what is done. See git-rebase(1) for a description of
+the autosquash feature."
+ (interactive)
+ (magit-commit-for-autosquash "squash!"))
+
(defun magit-edit-ignore-string (file)
"Prompt the user for the string to be ignored.
A list of predefined values with wildcards is derived from the
View
12 magit.texi
@@ -356,6 +356,15 @@ Typing @kbd{C} will also pop up the change description buffer, but in
addition, it will try to insert a ChangeLog-style entry for the change
that point is in.
+You can create fixup! or squash! commits for the autosquash feature of
+@code{git-rebase} by typing @kbd{' '} or @kbd{' s}, respectively. The
+created commit will be autosquashed to the commit at point or, if the commit
+at point is to be autosquashed, to the commit the commit at point will be
+squashed to. If there are staged changes, the new commit is created out of
+those. If there are no staged changes,
+@code{magit-commit-all-when-nothing-staged} determines what is done. Magit
+will offer to set rebase.autosquash option if necessary.
+
@node History
@chapter History
@@ -742,6 +751,9 @@ by pressing @kbd{C-c C-c}, which is bound to @code{server-edit} in
this mode, and you can abort the rebase with @kbd{C-c C-k}, just like
when editing a commit message in Magit.
+See @ref{Staging and Committing} for information about creating commits
+for the autosquash feature of @code{git-rebase}.
+
@node Rewriting
@chapter Rewriting
Something went wrong with that request. Please try again.