Skip to content
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

Edit all messages of consecutive squashed commits in one go #92

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 45 additions & 9 deletions gitrevise/todo.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import re
from enum import Enum
from typing import List, Optional
from typing import List, Optional, Tuple

from .odb import Commit, MissingObject, Repository
from .utils import cut_commit, edit_commit_message, run_editor, run_sequence_editor
Expand Down Expand Up @@ -242,27 +242,63 @@ def edit_todos(
return result


def is_fixup(todo: Step) -> bool:
return todo.kind in (StepKind.FIXUP, StepKind.SQUASH)
Comment on lines +245 to +246
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: Is "squash" or "fixup" the generalization of the two? I would say "squash", since you can say

I squashed my fixups.



def squash_message_template(
target_message: bytes, fixups: List[Tuple[StepKind, bytes]]
) -> bytes:
fused = (
b"# This is a combination of %d commits.\n" % (len(fixups) + 1)
+ b"# This is the 1st commit message:\n"
+ b"\n"
+ target_message
)

for index, (kind, message) in enumerate(fixups):
fused += b"\n"
if kind == StepKind.FIXUP:
fused += (
b"# The commit message #%d will be skipped:\n" % (index + 2)
+ b"\n"
+ b"".join(b"# " + line for line in message.splitlines(keepends=True))
)
else:
assert kind == StepKind.SQUASH
fused += b"# This is the commit message #%d:\n\n%s" % (index + 2, message)

return fused


def apply_todos(
current: Optional[Commit],
todos: List[Step],
reauthor: bool = False,
) -> Commit:
for step in todos:
fixups: List[Tuple[StepKind, bytes]] = []

for index, step in enumerate(todos):
rebased = step.commit.rebase(current).update(message=step.message)
if step.kind == StepKind.PICK:
current = rebased
elif step.kind == StepKind.FIXUP:
elif is_fixup(step):
if current is None:
raise ValueError("Cannot apply fixup as first commit")
if not fixups:
fixup_target_message = current.message
fixups.append((step.kind, rebased.message))
current = current.update(tree=rebased.tree())
is_last_fixup = index + 1 == len(todos) or not is_fixup(todos[index + 1])
if is_last_fixup:
if any(kind == StepKind.SQUASH for kind, message in fixups):
current = current.update(
message=squash_message_template(fixup_target_message, fixups)
)
current = edit_commit_message(current)
fixups.clear()
Comment on lines -254 to +299
Copy link
Contributor

@anordal anordal Jun 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Since you need to look ahead of at least squash and fixup steps, to see if there are more, you might check the same predicate unconditionally (for all step kinds) for the benefit of discovering the beginning of the sequence in the iteration it happens. Then, the first commit could go into the same list as the rest instead of having a separate variable. The same check negated:

is_squashed_into = index + 1 < len(todos) and is_fixup(todos[index + 1])

Do you know what else could use this? If a later commit that is squashed into this commit has a known tree (#133), we can also avoid raising any conflict on this commit, because this commit's tree is to be discarded.

This happens when part of commit A belongs in commit B: When you have carved out a fixup of B from A, you have the situation where "fixup B" comes before B. To preserve the metadata of the right commit, you may prefer to squash "fixup B" into B instead of the opposite. Hm, maybe this particular case is better supported by an actual "prefix" action that doesn't require the user to reorder and us to be smart about it.

elif step.kind == StepKind.REWORD:
current = edit_commit_message(rebased)
elif step.kind == StepKind.SQUASH:
if current is None:
raise ValueError("Cannot apply squash as first commit")
fused = current.message + b"\n\n" + rebased.message
current = current.update(tree=rebased.tree(), message=fused)
current = edit_commit_message(current)
elif step.kind == StepKind.CUT:
current = cut_commit(rebased)
elif step.kind == StepKind.INDEX:
Expand Down
115 changes: 115 additions & 0 deletions tests/test_fixup.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,3 +396,118 @@ def test_autosquash_multiline_summary(repo: Repository) -> None:
new = repo.get_commit("HEAD")
assert old != new, "commit was modified"
assert old.parents() == new.parents(), "parents are unchanged"


def test_autosquash_multiple_squashes() -> None:
bash(
"""
git commit --allow-empty -m 'initial commit'
git commit --allow-empty -m 'target'
git commit --allow-empty --squash :/^target --no-edit
git commit --allow-empty --squash :/^target --no-edit
"""
)

with editor_main([":/^init", "--autosquash"], input=b"") as ed:
with ed.next_file() as f:
assert f.startswith_dedent(
"""\
# This is a combination of 3 commits.
# This is the 1st commit message:

target

# This is the commit message #2:

squash! target

# This is the commit message #3:

squash! target
"""
)
f.replace_dedent("two squashes")


def test_autosquash_squash_and_fixup() -> None:
bash(
"""
git commit --allow-empty -m 'initial commit'
git commit --allow-empty -m 'fixup-target'
git commit --allow-empty --fixup :/^fixup-target --no-edit
git commit --allow-empty -m 'squash-target'
git commit --allow-empty --squash :/^squash-target --no-edit
"""
)

with editor_main([":/^init", "--autosquash"], input=b"") as ed:
with ed.next_file() as f:
assert f.startswith_dedent(
"""\
# This is a combination of 2 commits.
# This is the 1st commit message:

squash-target

# This is the commit message #2:

squash! squash-target
"""
)
f.replace_dedent("squash + fixup")


def test_autosquash_multiple_squashes_with_fixup() -> None:
bash(
"""
git commit --allow-empty -m 'initial commit'
git commit --allow-empty -m 'target'
git commit --allow-empty --squash :/^target --no-edit
git commit --allow-empty --fixup :/^target --no-edit
git commit --allow-empty --squash :/^target --no-edit
git commit --allow-empty -m 'unrelated'
"""
)
with editor_main([":/^init", "--autosquash"], input=b"") as ed:
with ed.next_file() as f:
assert f.startswith_dedent(
"""\
# This is a combination of 4 commits.
# This is the 1st commit message:

target

# This is the commit message #2:

squash! target

# The commit message #3 will be skipped:

# fixup! target

# This is the commit message #4:

squash! target
"""
)
f.replace_dedent("squashes + fixup")


def test_autosquash_multiple_independent_squashes() -> None:
bash(
"""
git commit --allow-empty -m 'initial commit'
git commit --allow-empty -m 'target1'
git commit --allow-empty -m 'target2'
git commit --allow-empty --squash :/^target1 --no-edit
git commit --allow-empty --squash :/^target2 --no-edit
"""
)

with editor_main([":/^init", "--autosquash"], input=b"") as ed:
with ed.next_file() as f:
assert f.startswith_dedent("# This is a combination of 2 commits.")
f.replace_dedent("squash 1")
with ed.next_file() as f:
assert f.startswith_dedent("# This is a combination of 2 commits.")
f.replace_dedent("squash 2")