| Message ID | 20220227220924.2144325-1-gitster@pobox.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 2587df669bff9daeb7d2a66cfce6b1ce28af2ef3 |
| Headers | show |
| Series | [v2] rerere-train: two fixes to the use of "git show -s" | expand |
On Sun, Feb 27, 2022 at 02:09:24PM -0800, Junio C Hamano wrote: > The script uses "git show -s" to display the title of the merge > commit being studied, without explicitly disabling the pager, which > is not a safe thing to do in a script. > > For example, when the pager is set to "less" with "-SF" options (-S > tells the pager not to fold lines but allow horizontal scrolling to > show the overly long lines, -F tells the pager not to wait if the > output in its entirety is shown on a single page), and the title of > the merge commit is longer than the width of the terminal, the pager > will wait until the end-user tells it to quit after showing the > single line. > > Explicitly disable the pager with this "git show" invocation to fix > this. > > The command uses the "--pretty=format:..." format, which adds LF in > between each pair of commits it outputs, which means that the label > for the merge being learned from will be followed by the next > message on the same line. "--pretty=tformat:..." is what we should > instead, which adds LF after each commit, or a more modern way to > spell it, i.e. "--format=...". This existing breakage becomes > easier to see, now we no longer use the pager. Sounds good (definitely better than two separate commits). > > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > > * Relative to the initial version, the "--no-merges" change has it was "--merges", not "--no-merges" > been removed because the end user can still give --merges from > the command line and the filtering of merges done by the script > is still needed for correctness. You probably mean that the user can pass "--no-merges HEAD" but that would just make the effective command git rev-list --merges --no-merges HEAD which outputs nothing. I don't think `git rev-list --merges "$@"` will ever output non-merge commits, so the filtering should not be necessary. > > contrib/rerere-train.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/contrib/rerere-train.sh b/contrib/rerere-train.sh > index 75125d6ae0..26b724c8c6 100755 > --- a/contrib/rerere-train.sh > +++ b/contrib/rerere-train.sh > @@ -86,7 +86,7 @@ do > fi > if test -s "$GIT_DIR/MERGE_RR" > then > - git show -s --pretty=format:"Learning from %h %s" "$commit" > + git --no-pager show -s --format="Learning from %h %s" "$commit" > git rerere > git checkout -q $commit -- . > git rerere > -- > 2.35.1-354-g715d08a9e5 >
diff --git a/contrib/rerere-train.sh b/contrib/rerere-train.sh index 75125d6ae0..26b724c8c6 100755 --- a/contrib/rerere-train.sh +++ b/contrib/rerere-train.sh @@ -86,7 +86,7 @@ do fi if test -s "$GIT_DIR/MERGE_RR" then - git show -s --pretty=format:"Learning from %h %s" "$commit" + git --no-pager show -s --format="Learning from %h %s" "$commit" git rerere git checkout -q $commit -- . git rerere
The script uses "git show -s" to display the title of the merge commit being studied, without explicitly disabling the pager, which is not a safe thing to do in a script. For example, when the pager is set to "less" with "-SF" options (-S tells the pager not to fold lines but allow horizontal scrolling to show the overly long lines, -F tells the pager not to wait if the output in its entirety is shown on a single page), and the title of the merge commit is longer than the width of the terminal, the pager will wait until the end-user tells it to quit after showing the single line. Explicitly disable the pager with this "git show" invocation to fix this. The command uses the "--pretty=format:..." format, which adds LF in between each pair of commits it outputs, which means that the label for the merge being learned from will be followed by the next message on the same line. "--pretty=tformat:..." is what we should instead, which adds LF after each commit, or a more modern way to spell it, i.e. "--format=...". This existing breakage becomes easier to see, now we no longer use the pager. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- * Relative to the initial version, the "--no-merges" change has been removed because the end user can still give --merges from the command line and the filtering of merges done by the script is still needed for correctness. contrib/rerere-train.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)