Ann T Ropea <bedhan...@gmx.de> writes:
> v6: polish to take Junio's comments from
> <xmqqshcqmoe7....@gitster.mtv.corp.google.com> into account
> t/t2020-checkout-detach.sh | 114
> ++++++++++++++++++++++++++++++++++++++++++
> ...
Thanks; with this one replaced, I'd expect that poisoned gettext
test to pass now.
I saw some style issues, so I'll queue a tentative fix-up on top.
> + # The first detach operation is more chatty than the following ones.
> + cat 1>1st_detach <<'EOF' &&
> +Note: checking out 'HEAD^'.
> +
> +You are in 'detached HEAD' state. You can look around, make experimental
> +changes and commit them, and you can discard any commits you make in this
> +state without impacting any branches by performing another checkout.
> +
> +If you want to create a new branch to retain commits you create, you may
> +do so (now or later) by using -b with the checkout command again. Example:
> +
> + git checkout -b <new-branch-name>
> +
> +HEAD is now at 7c7cd714e262 three
> +EOF
It looks somewhat strange to explicitly say 1> when redirecting the
standard output. Also we prefer to indent our here-doc to the same
depth as commands by using "<<-", i.e.
cat >1st_detach <<-'EOF' &&
Note: checking out 'HEAD^'.
...
EOF
> + reset && check_not_detached && sane_unset GIT_PRINT_SHA1_ELLIPSIS &&
It also was a bit irritating to see multiple commands form an
overlong single line, like this one.
> + test_i18ncmp 1st_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS &&
> +
> + GIT_PRINT_SHA1_ELLIPSIS="no" git -c 'core.abbrev=12' checkout HEAD^
> 1>actual 2>&1 &&
> + check_detached &&
> + test_i18ncmp 2nd_detach actual && sane_unset GIT_PRINT_SHA1_ELLIPSIS &&
This part uses "set and export only for a single invocation of git",
and because the variable is unset at the end of the previous step
after 1st_detach and actual are compared, this unset at the end
feels redundant.
> + GIT_PRINT_SHA1_ELLIPSIS='nope' && export GIT_PRINT_SHA1_ELLIPSIS && git
> -c 'core.abbrev=12' checkout HEAD^ 1>actual 2>&1 &&
But now this part sets and exports the variable for the remainder of
the script (until it is unset).
Is the use of these two styles, i.e.
VAR=value && export VAR && git -c core.abbrev=12 subcmd
VAR=value git -c core.abbrev=12 subcmd
intended? If so for what purpose? It's not like we are testing if
the shell implements environment variables correctly, so I'd expect
that the result would be easier to follow if it stuck to a single
style and used it consistently.
