-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add command to find base commit for creating a fixup #3105
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
# Fixup Commits | ||
|
||
## Background | ||
|
||
There's this common scenario that you have a PR in review, the reviewer is | ||
requesting some changes, and you make those changes and would normally simply | ||
squash them into the original commit that they came from. If you do that, | ||
however, there's no way for the reviewer to see what you changed. You could just | ||
make a separate commit with those changes at the end of the branch, but this is | ||
not ideal because it results in a git history that is not very clean. | ||
|
||
To help with this, git has a concept of fixup commits: you do make a separate | ||
commit, but the subject of this commit is the string "fixup! " followed by the | ||
original commit subject. This both tells the reviewer what's going on (you are | ||
making a change that you later will squash into the designated commit), and it | ||
provides an easy way to actually perform this squash operation when you are | ||
ready to do that (before merging). | ||
|
||
## Creating fixup commits | ||
|
||
You could of course create fixup commits manually by typing in the commit | ||
message with the prefix yourself. But lazygit has an easier way to do that: | ||
in the Commits view, select the commit that you want to create a fixup for, and | ||
press shift-F (for "Create fixup commit for this commit"). This automatically | ||
creates a commit with the appropriate subject line. | ||
|
||
Don't confuse this with the lowercase "f" command ("Fixup commit"); that one | ||
squashes the selected commit into its parent, this is not what we want here. | ||
|
||
## Squashing fixup commits | ||
|
||
When you're ready to merge the branch and want to squash all these fixup commits | ||
that you created, that's very easy to do: select the first commit of your branch | ||
and hit shift-S (for "Squash all 'fixup!' commits above selected commit | ||
(autosquash)"). Boom, done. | ||
|
||
## Finding the commit to create a fixup for | ||
|
||
When you are making changes to code that you changed earlier in a long branch, | ||
it can be tedious to find the commit to squash it into. Lazygit has a command to | ||
help you with this, too: in the Files view, press ctrl-f to select the right | ||
base commit in the Commits view automatically. From there, you can either press | ||
shift-F to create a fixup commit for it, or shift-A to amend your changes into | ||
the commit if you haven't published your branch yet. | ||
|
||
This command works in many cases, and when it does it almost feels like magic, | ||
but it's important to understand its limitations because it doesn't always work. | ||
The way it works is that it looks at the deleted lines of your current | ||
modifications, blames them to find out which commit those lines come from, and | ||
if they all come from the same commit, it selects it. So here are cases where it | ||
doesn't work: | ||
|
||
- Your current diff has only added lines, but no deleted lines. In this case | ||
there's no way for lazygit to know which commit you want to add them to. | ||
- The deleted lines belong to multiple different commits. In this case you can | ||
help lazygit by staging a set of files or hunks that all belong to the same | ||
commit; if some changes are staged, the ctrl-f command works only on those. | ||
- The found commit is already on master; in this case, lazygit refuses to select | ||
it, because it doesn't make sense to create fixups for it, let alone amend to | ||
it. | ||
|
||
To sum it up: the command works great if you are changing code again that you | ||
changed or added earlier in the same branch. This is a common enough case to | ||
make the command useful. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
package git_commands | ||
|
||
import ( | ||
"fmt" | ||
) | ||
|
||
type BlameCommands struct { | ||
*GitCommon | ||
} | ||
|
||
func NewBlameCommands(gitCommon *GitCommon) *BlameCommands { | ||
return &BlameCommands{ | ||
GitCommon: gitCommon, | ||
} | ||
} | ||
|
||
// Blame a range of lines. For each line, output the hash of the commit where | ||
// the line last changed, then a space, then a description of the commit (author | ||
// and date), another space, and then the line. For example: | ||
// | ||
// ac90ebac688fe8bc2ffd922157a9d2c54681d2aa (Stefan Haller 2023-08-01 14:54:56 +0200 11) func NewBlameCommands(gitCommon *GitCommon) *BlameCommands { | ||
// ac90ebac688fe8bc2ffd922157a9d2c54681d2aa (Stefan Haller 2023-08-01 14:54:56 +0200 12) return &BlameCommands{ | ||
// ac90ebac688fe8bc2ffd922157a9d2c54681d2aa (Stefan Haller 2023-08-01 14:54:56 +0200 13) GitCommon: gitCommon, | ||
func (self *BlameCommands) BlameLineRange(filename string, commit string, firstLine int, numLines int) (string, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd add a comment giving an example output from the function There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added in 17616fb |
||
cmdArgs := NewGitCmd("blame"). | ||
Arg("-l"). | ||
Arg(fmt.Sprintf("-L%d,+%d", firstLine, numLines)). | ||
Arg(commit). | ||
Arg("--"). | ||
Arg(filename) | ||
|
||
return self.cmd.New(cmdArgs.ToArgv()).RunWithOutput() | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -198,6 +198,20 @@ func (self *CommitCommands) GetCommitMessagesFirstLine(shas []string) (string, e | |
return self.cmd.New(cmdArgs).DontLog().RunWithOutput() | ||
} | ||
|
||
// Example output: | ||
// | ||
// cd50c79ae Preserve the commit message correctly even if the description has blank lines | ||
// 3ebba5f32 Add test demonstrating a bug with preserving the commit message | ||
// 9a423c388 Remove unused function | ||
func (self *CommitCommands) GetShasAndCommitMessagesFirstLine(shas []string) (string, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added in 28373b0 |
||
cmdArgs := NewGitCmd("show"). | ||
Arg("--no-patch", "--pretty=format:%h %s"). | ||
Arg(shas...). | ||
ToArgv() | ||
|
||
return self.cmd.New(cmdArgs).DontLog().RunWithOutput() | ||
} | ||
|
||
func (self *CommitCommands) GetCommitsOneline(shas []string) (string, error) { | ||
cmdArgs := NewGitCmd("show"). | ||
Arg("--no-patch", "--oneline"). | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -64,6 +64,12 @@ func (self *FilesController) GetKeybindings(opts types.KeybindingsOpts) []*types | |
Handler: self.c.Helpers().WorkingTree.HandleCommitEditorPress, | ||
Description: self.c.Tr.CommitChangesWithEditor, | ||
}, | ||
{ | ||
Key: opts.GetKey(opts.Config.Files.FindBaseCommitForFixup), | ||
Handler: self.c.Helpers().FixupHelper.HandleFindBaseCommitForFixupPress, | ||
Description: self.c.Tr.FindBaseCommitForFixup, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs a tooltip: I can imagine seeing this description and not really knowing what it was talking about. It could be something simple like 'Find the commit that your current changes are building upon, for the sake of amending/fixing up the commit. This spares you from having to look through your branch's commits one-by-one to see which commit should be amended/fixed up. See docs: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added in 330151c |
||
Tooltip: self.c.Tr.FindBaseCommitForFixupTooltip, | ||
}, | ||
{ | ||
Key: opts.GetKey(opts.Config.Universal.Edit), | ||
Handler: self.checkSelectedFileNode(self.edit), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great writeup