From: Đoàn Trần Công Danh Date: Fri, 17 Jul 2020 00:25:00 GMT Subject: Re: [PATCH v6 2/5] rebase -i: support --committer-date-is-author-date Message-ID: <20200717002500.GA20467@danh.dev> In-Reply-To: <640a3f1b-4e00-4b8a-9a9f-207ea76c648a@gmail.com> On 2020-07-16 18:34:51+0100, Phillip Wood wrote: > >> That translation isn't correct either. > >> It seems like it's recurring pattern. > >> I'll take it to the Vietnamese translation team. > >> > >> Anyway, I've checked with other translation that I can understand > >> in part. I think > >> > >>     invalid ident line: %s > >> > >> is better candidate for the message. > > > > Yes I realized after sending my email that the string we're printing is > > NUL terminated so we don't need to specify the length with '*'. I think > > the best thing would be to change the message in this patch to 'invalid > > ident line: %s' and then have a follow up after this is merged to change > > all the "invalid ident line" messages to use "identity" instead. Would > > you be interested in taking on the follow up patch? > > In the end I decided it was better just to change the message in this > patch to something more descriptive. We can update the other commands > separately. For format-patch we should probably update the option > description for `--from` as well as the error message. Yes, I also think we should leave the other messages there for now. When this topic get merged, we will come back to see which one should be updated/changed. All other parts look sane to me. I don't mind get my hand wet if noone steps in. Thanks, Danh > > Best Wishes > > Phillip > > > > > Best Wishes > > > > Phillip > > > >> Since Spanish translation also mis-translates the message: > >> > >>     es.po:9836:msgid "invalid ident line: %.*s" > >>     es.po-9837-msgstr "sangría no válida: %.*s" > >> > >> "sangría" also means "indentation" in this context. > >> > >> Thanks, > >> -Danh > >> > >>> > >>> Best Wishes > >>> > >>> Phillip > >>> > >>>> > >>>>> +            goto out; > >>>>> +        } > >>>>> +        if (!ident.date_begin) { > >>>>> +            res = error(_("corrupted author without date > >>>>> information")); > >>>>> +            goto out; > >>>>> +        } > >>>>> + > >>>>> +        strbuf_addf(&date, "@%.*s %.*s", > >>>>> +                (int)(ident.date_end - ident.date_begin), > >>>>> +                ident.date_begin, > >>>>> +                (int)(ident.tz_end - ident.tz_begin), > >>>>> +                ident.tz_begin); > >>>>> +        res = setenv("GIT_COMMITTER_DATE", date.buf, 1); > >>>>> +        strbuf_release(&date); > >>>>> + > >>>>> +        if (res) > >>>>> +            goto out; > >>>>> +    } > >>>>> + > >>>>>        if (write_index_as_tree(&tree, r->index, r->index_file, 0, > >>>>> NULL)) { > >>>>>            res = error(_("git write-tree failed to write a tree")); > >>>>>            goto out; > >>>>> @@ -2532,6 +2578,11 @@ static int read_populate_opts(struct > >>>>> replay_opts *opts) > >>>>>                opts->signoff = 1; > >>>>>            } > >>>>> +        if (file_exists(rebase_path_cdate_is_adate())) { > >>>>> +            opts->allow_ff = 0; > >>>>> +            opts->committer_date_is_author_date = 1; > >>>>> +        } > >>>>> + > >>>>>            if (file_exists(rebase_path_reschedule_failed_exec())) > >>>>>                opts->reschedule_failed_exec = 1; > >>>>> @@ -2622,6 +2673,8 @@ int write_basic_state(struct replay_opts > >>>>> *opts, const char *head_name, > >>>>>            write_file(rebase_path_drop_redundant_commits(), "%s", ""); > >>>>>        if (opts->keep_redundant_commits) > >>>>>            write_file(rebase_path_keep_redundant_commits(), "%s", ""); > >>>>> +    if (opts->committer_date_is_author_date) > >>>>> +        write_file(rebase_path_cdate_is_adate(), "%s", ""); > >>>>>        if (opts->reschedule_failed_exec) > >>>>>            write_file(rebase_path_reschedule_failed_exec(), "%s", ""); > >>>>> @@ -3542,6 +3595,10 @@ static int do_merge(struct repository *r, > >>>>>                goto leave_merge; > >>>>>            } > >>>>> +        if (opts->committer_date_is_author_date) > >>>>> +            argv_array_pushf(&cmd.env_array, "GIT_COMMITTER_DATE=%s", > >>>>> +                     author_date_from_env_array(&cmd.env_array)); > >>>>> + > >>>>>            cmd.git_cmd = 1; > >>>>>            argv_array_push(&cmd.args, "merge"); > >>>>>            argv_array_push(&cmd.args, "-s"); > >>>>> @@ -3819,7 +3876,8 @@ static int pick_commits(struct repository *r, > >>>>>        setenv(GIT_REFLOG_ACTION, action_name(opts), 0); > >>>>>        if (opts->allow_ff) > >>>>>            assert(!(opts->signoff || opts->no_commit || > >>>>> -                opts->record_origin || opts->edit)); > >>>>> +                opts->record_origin || opts->edit || > >>>>> +                opts->committer_date_is_author_date)); > >>>>>        if (read_and_refresh_cache(r, opts)) > >>>>>            return -1; > >>>>> diff --git a/sequencer.h b/sequencer.h > >>>>> index 0bee85093e..4ab94119ae 100644 > >>>>> --- a/sequencer.h > >>>>> +++ b/sequencer.h > >>>>> @@ -45,6 +45,7 @@ struct replay_opts { > >>>>>        int verbose; > >>>>>        int quiet; > >>>>>        int reschedule_failed_exec; > >>>>> +    int committer_date_is_author_date; > >>>>>        int mainline; > >>>>> diff --git a/t/t3422-rebase-incompatible-options.sh > >>>>> b/t/t3422-rebase-incompatible-options.sh > >>>>> index 55ca46786d..c8234062c6 100755 > >>>>> --- a/t/t3422-rebase-incompatible-options.sh > >>>>> +++ b/t/t3422-rebase-incompatible-options.sh > >>>>> @@ -61,7 +61,6 @@ test_rebase_am_only () { > >>>>>    } > >>>>>    test_rebase_am_only --whitespace=fix > >>>>> -test_rebase_am_only --committer-date-is-author-date > >>>>>    test_rebase_am_only -C4 > >>>>>    test_expect_success REBASE_P '--preserve-merges incompatible > >>>>> with --signoff' ' > >>>>> diff --git a/t/t3436-rebase-more-options.sh > >>>>> b/t/t3436-rebase-more-options.sh > >>>>> index 4f8a6e51c9..50a63d8ebe 100755 > >>>>> --- a/t/t3436-rebase-more-options.sh > >>>>> +++ b/t/t3436-rebase-more-options.sh > >>>>> @@ -9,6 +9,9 @@ test_description='tests to ensure compatibility > >>>>> between am and interactive backe > >>>>>    . "$TEST_DIRECTORY"/lib-rebase.sh > >>>>> +GIT_AUTHOR_DATE="1999-04-02T08:03:20+05:30" > >>>>> +export GIT_AUTHOR_DATE > >>>>> + > >>>>>    # This is a special case in which both am and interactive backends > >>>>>    # provide the same output. It was done intentionally because > >>>>>    # both the backends fall short of optimal behaviour. > >>>>> @@ -21,11 +24,20 @@ test_expect_success 'setup' ' > >>>>>        test_write_lines "line 1" "new line 2" "line 3" >file && > >>>>>        git commit -am "update file" && > >>>>>        git tag side && > >>>>> +    test_commit commit1 foo foo1 && > >>>>> +    test_commit commit2 foo foo2 && > >>>>> +    test_commit commit3 foo foo3 && > >>>>>        git checkout --orphan master && > >>>>> +    rm foo && > >>>>>        test_write_lines "line 1" "        line 2" "line 3" >file && > >>>>>        git commit -am "add file" && > >>>>> -    git tag main > >>>>> +    git tag main && > >>>>> + > >>>>> +    mkdir test-bin && > >>>>> +    write_script test-bin/git-merge-test <<-\EOF > >>>>> +    exec git-merge-recursive "$@" > >>>>> +    EOF > >>>>>    ' > >>>>>    test_expect_success '--ignore-whitespace works with apply > >>>>> backend' ' > >>>>> @@ -52,6 +64,50 @@ test_expect_success '--ignore-whitespace is > >>>>> remembered when continuing' ' > >>>>>        git diff --exit-code side > >>>>>    ' > >>>>> +test_ctime_is_atime () { > >>>>> +    git log $1 --format=%ai >authortime && > >>>>> +    git log $1 --format=%ci >committertime && > >>>>> +    test_cmp authortime committertime > >>>>> +} > >>>>> + > >>>>> +test_expect_success '--committer-date-is-author-date works with > >>>>> apply backend' ' > >>>>> +    GIT_AUTHOR_DATE="@1234 +0300" git commit --amend > >>>>> --reset-author && > >>>>> +    git rebase --apply --committer-date-is-author-date HEAD^ && > >>>>> +    test_ctime_is_atime -1 > >>>>> +' > >>>>> + > >>>>> +test_expect_success '--committer-date-is-author-date works with > >>>>> merge backend' ' > >>>>> +    GIT_AUTHOR_DATE="@1234 +0300" git commit --amend > >>>>> --reset-author && > >>>>> +    git rebase -m --committer-date-is-author-date HEAD^ && > >>>>> +    test_ctime_is_atime -1 > >>>>> +' > >>>>> + > >>>>> +test_expect_success '--committer-date-is-author-date works with > >>>>> rebase -r' ' > >>>>> +    git checkout side && > >>>>> +    GIT_AUTHOR_DATE="@1234 +0300" git merge --no-ff commit3 && > >>>>> +    git rebase -r --root --committer-date-is-author-date && > >>>>> +    test_ctime_is_atime > >>>>> +' > >>>>> + > >>>>> +test_expect_success '--committer-date-is-author-date works when > >>>>> forking merge' ' > >>>>> +    git checkout side && > >>>>> +    GIT_AUTHOR_DATE="@1234 +0300" git merge --no-ff commit3 && > >>>>> +    PATH="./test-bin:$PATH" git rebase -r --root --strategy=test \ > >>>>> +                    --committer-date-is-author-date && > >>>>> +    test_ctime_is_atime > >>>>> +' > >>>>> + > >>>>> +test_expect_success '--committer-date-is-author-date works when > >>>>> committing conflict resolution' ' > >>>>> +    git checkout commit2 && > >>>>> +    GIT_AUTHOR_DATE="@1980 +0000" git commit --amend --only > >>>>> --reset-author && > >>>>> +    test_must_fail git rebase -m --committer-date-is-author-date \ > >>>>> +        --onto HEAD^^ HEAD^ && > >>>>> +    echo resolved > foo && > >>>> > >>>> Nitpick: no space after ">" :D > >>>> > >> > -- Danh