On Mon, Mar 01 2021, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
>
>> @@ -220,6 +225,9 @@ test_commit () {
>> --no-tag)
>> no_tag=yes
>> ;;
>> + --annotated-tag)
>> + annotated_tag=yes
>> + ;;
>
> A new option is welcome, but shouldn't we be straightening out the
> variables that keep track of the options around tagging? It's not
> like it is possible to have 4 (two variables that can independently
> set to 'yes') possibilities, so something along the lines of ...
>
> test_commit () {
> + tag=light &&
> notick= &&
> ...
> while test $# != 0
> do
> case "$1" in
> ...
> --no-tag)
> - no_tag=yes
> + tag=none
> ;;
> + --annotated)
> + tag=annotated
> + ;;
> ...
> - if test -z "$no_tag"
> - then
> + case "$tag" in
> + none)
> + ;;
> + light)
> git ${indir:+ -C "$indir") tag "${4:-$1}"
> + ;;
> + annotated)
> + git ${indir:+ -C "$indir") tag -m "$1" "${4:-$1}"
> + ;;
> + esac
> ...
>
> after this step (meaning, we may want to start from fixing the no_tag=yes
> to tag=none before introducing this new feature).
Yeah, as noted in the last paragraph of the commit message:
The placement of --annotated-tag after "notick" in the case of the
documentation, and then after "no_tag" in the case of the code is
slightly inconsistent. It's to evade a merge conflict with two other
commits adding a --printf option, and another one adding documentation
for --no-tag.
So the existing patch + not doing a bigger refactoring is because I
didn't want to cause conflicts for you to solve with other in-flight
topics. It would be easier with ab/pickaxe-pcre2 merged down :)
I'd prefer to keep this as-is for now, and just refactor this function a
bit after the current topics land.
In particular I'd like to make the the "message file contents tags"
arguments optionally have --long-option versions, so e.g. tests that
need a separate --commit-message and --tag-message can use the helper,
there's also quite a few that could use a --file=<existing file>
v.s. echo-ing a "message" to a "file".
>> @@ -244,7 +252,15 @@ test_commit () {
>> $signoff -m "$1" &&
>> if test -z "$no_tag"
>> then
>> - git ${indir:+ -C "$indir"} tag "${4:-$1}"
>> + if test -n "$annotated_tag"
>> + then
>> + if test -z "$notick"
>> + then
>> + test_tick
>> + fi &&
>> + test_tick
>> + fi &&
>> + git ${indir:+ -C "$indir"} tag ${annotated_tag:+ -a -m "$1"} "${4:-$1}"
>> fi
>> }
