Re: [PATCH 2/3] t/lib-rebase: set_fake_editor(): handle FAKE_LINES more consistently
From: Junio C Hamano <hidden>
Date: 2023-08-07 19:09:12
Oswald Buddenhagen [off-list ref] writes:
Default next action after 'fakesha' to preserving the command instead
of forcing 'pick', consistently with other "instant-effect" keywords.
There is no reason why one would want that inconsistency, so this was
clearly just an oversight in commit 5dcdd740 ("t/lib-rebase: prepare
for testing `git rebase --rebase-merges`"). Rectifying it makes the
behavior easier to reason about and document.
This would affect hypothetical "fakesha <n>" sequences where line <n>
already isn't a pick, which currently don't appear.
Signed-off-by: Oswald Buddenhagen <redacted>
---
Cc: Phillip Wood <redacted>
---
t/lib-rebase.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)I do recall seeing this change and remember wondering what the fallout from this change would be. So relative to the previous round, the above is a definite improvement to clearly state that no test that is currently in the codebase is affected by this change. As to the change itself, I do not much care among (1) what this patch does, (2) doing nothing, or (3) barf when the action is not a pick. At least, having this step separate from other changes like this round of the series does is a very good thing---when somebody with more knowledge and stake in what the fake-editor does appears and explains why forcing pick is a good idea, we can easily revert only this step. Will queue as-is together with the other two patches. Thanks.
quoted hunk
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index e6179ab529..9ed87ca7ab 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh@@ -64,7 +64,7 @@ set_fake_editor () { fakesha) test \& != "$action" || action=pick echo "$action XXXXXXX False commit" >> "$1" - action=pick;; + action=\&;; *) sed -n "${line}s/^[a-z][a-z]*/$action/p" < "$1".tmp >> "$1" action=\&;;