From: Phillip Wood Date: Thu, 16 Jul 2020 08:23:17 GMT Subject: Re: [PATCH v6 2/5] rebase -i: support --committer-date-is-author-date Message-ID: <281d92d3-f901-bca1-6030-fd0c8761f214@gmail.com> In-Reply-To: <20200715142717.GA7803@danh.dev> Hi Danh On 15/07/2020 15:27, Đoàn Trần Công Danh wrote: > > Hi Phillip, > > On 2020-07-13 11:10:42+0100, Phillip Wood wrote: >> [...] >> >> + if (opts->committer_date_is_author_date) { >> + struct ident_split ident; >> + struct strbuf date = STRBUF_INIT; >> + >> + if (split_ident_line(&ident, author, (int)strlen(author)) < 0) { >> + res = error(_("malformed ident line '%s'"), author); > > I've checked with the translation for my native language (vi). > The translators seem to misread ident (as in identity) as > indent (as in indentation). > > The translation in po/vi.po:25045 (of v2.28.0-rc0) reads: > > #~ msgid "malformed ident line" > #~ msgstr "thụt đầu dòng dị hình" > > Translating back to English, it reads: "malformed indentation". > > Hence, I think it would read better if we write: > > res = error(_("malformed identity line '%s'"), author); > > 3 more characters is not that much :) Looking through the existing strings "invalid ident line: %.*s" is already used by am so maybe we should reuse that (log.c also contains "invalid ident line: %s"). Is the translation correct? #: builtin/am.c:1270 #, c-format msgid "invalid ident line: %.*s" msgstr "dòng thụt lề không hợp lệ: %.*s" 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 >