| Message ID | xmqqy21w3z78.fsf_-_@gitster.g (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Re* [PATCH] rerere-train: modernise a bit | expand |
On Sun, Feb 27, 2022 at 11:07:55AM -0800, Junio C Hamano wrote: > Johannes Altmanninger <aclopte@gmail.com> writes: > > > Yep, tformat is more correct semantically, but it's worth noting that there > > is no behavior change here. These commands behave the same > > > > git show -s --pretty=tformat:"Learning" HEAD > > git show -s --pretty=format:"Learning" HEAD > > Your observation is not quite right. > > The difference between tformat and format does matter in practice, > unless your pager is hiding the difference. Right, I forgot about the pager. Both patches LGTM then. The --no-pager fix would have prevented my confusion, which is an argument for placing it first. > > $ export GIT_PAGER=cat; # disable the pager > $ git show -s --pretty=format:"%s" HEAD; echo Q > The eighth batchQ > $ exit > > This episode also exposes another bug in the rerere-train script, > caused by the fact that it lets GIT_PAGER to interfere. > > --- >8 --- > Subject: rerere-train: prevent GIT_PAGER from pausing 'git show -s' > > The script uses "git show -s --format" 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 for this "git show" invocation to avoid > this. > > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > contrib/rerere-train.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git c/contrib/rerere-train.sh w/contrib/rerere-train.sh > index 499b07e4a6..2b9df7b6f2 100755 > --- c/contrib/rerere-train.sh > +++ w/contrib/rerere-train.sh > @@ -81,7 +81,7 @@ do > fi > if test -s "$GIT_DIR/MERGE_RR" > then > - git show -s --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
Johannes Altmanninger <aclopte@gmail.com> writes: > The --no-pager fix would have prevented my confusion, which is an argument > for placing it first. I would rather squash them into one. "--no-pager" alone would give an apparent regression to people like you whose pager "corrected" the output from the command, but with two changes squashed together, we would not have to see any regression. Thanks.
diff --git c/contrib/rerere-train.sh w/contrib/rerere-train.sh index 499b07e4a6..2b9df7b6f2 100755 --- c/contrib/rerere-train.sh +++ w/contrib/rerere-train.sh @@ -81,7 +81,7 @@ do fi if test -s "$GIT_DIR/MERGE_RR" then - git show -s --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