Edit all messages of consecutive squashed commits in one go #92
Conversation
krobelus
force-pushed
the
edit-consecutive-squashes
branch
from
13278f3
to
25a0bb6
Compare
krobelus
force-pushed
the
edit-consecutive-squashes
branch
from
25a0bb6
to
fe4968c
Compare
|
this broke sequences like Still it has the potential to break scripts that override GIT_EDITOR to edit the commit message, but that risk is fairly low. |
| 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() |
There was a problem hiding this comment.
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.
| def is_fixup(todo: Step) -> bool: | ||
| return todo.kind in (StepKind.FIXUP, StepKind.SQUASH) |
There was a problem hiding this comment.
Nitpick: Is "squash" or "fixup" the generalization of the two? I would say "squash", since you can say
I squashed my fixups.
Closes #77
The CI has some unrelated false positives, which will be fixed in Pylint 2.9 ("Suppress consider-using-with inside context managers.")