Re: [PATCH] tag: add --edit option

[Nicolas Morey-Chaisemartin <NMoreyChaisemartin@suse.de>]

Le 01/02/2018 à 11:16, Eric Sunshine a écrit :
> On Thu, Feb 1, 2018 at 4:49 AM, Nicolas Morey-Chaisemartin
> <nmoreychaisemartin@suse.com> wrote:
>> Add a --edit option whichs allows modifying the messages provided by -m or -F,
>> the same way git commit --edit does.
>>
>> Signed-off-by: Nicolas Morey-Chaisemartin <NMoreyChaisemartin@suse.com>
>> ---
>> diff --git a/builtin/tag.c b/builtin/tag.c
>> @@ -224,7 +225,7 @@ static void create_tag(const struct object_id *object, const char *tag,
>> -       if (!opt->message_given) {
>> +       if (!opt->message_given || opt->use_editor) {
>>
>> -               if (!is_null_oid(prev)) {
>> +               if (opt->message_given) {
>> +                       write_or_die(fd, buf->buf, buf->len);
>> +                       strbuf_reset(buf);
>> +               } else if (!is_null_oid(prev)) {
>>                         write_tag_body(fd, prev);
>>                 } else {
> A little below this change is where launch_editor() is actually
> invoked. If it fails for some reason, it prints:
>
>     Please supply the message using either -m or -F option.
>
> which seems a bit counterintuitive if the user *did* specify one of
> those options along with --edit. I wonder if that message needs to be
> adjusted.
>
Yes I'll fix this.

>> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
>> @@ -452,6 +452,23 @@ test_expect_success \
>> +get_tag_header annotated-tag-edit $commit commit $time >expect
>> +echo "An edited message" >>expect
>> +test_expect_success 'set up editor' '
>> +       cat >editor <<-\EOF &&
>> +       #!/bin/sh
>> +       sed -e "s/A message/An edited message/g" <"$1" >"$1-"
>> +       mv "$1-" "$1"
>> +       EOF
>> +       chmod 755 editor
> If you use write_script() to create the fake editor, then it supplies
> the #!/bin/sh line for you and does the 'chmod', so you only need to
> supply the actual script payload. Also, other "editors" in this test
> file are named "fakeeditor", so perhaps follow suit.
>
>     write_script fakeeditor <<-\EOF
>         sed -e "s/A message/An edited message/g" <"$1" >"$1-"
>         mv "$1-" "$1"
>     EOF
>
I dumbly copied the test from commit --edit as it was my reference.
I'll fix the names and switch to write_script.

Thanks

Nicolas