| Message ID | 20240228-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-v6-0-8ec34c052b39@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Implement `git log --merge` also for rebase/cherry-pick/revert | expand |
Hi Philippe On 28/02/2024 13:54, Philippe Blain wrote: > Range-diff versus v5: > > 1: c9536431d1 ! 1: 363657561c revision: ensure MERGE_HEAD is a ref in prepare_show_merge > @@ Commit message > (2) obtain the oid without any prefixing by refs.c:repo_dwim_ref() > (3) error out when MERGE_HEAD is a symref. > > + Note that we avoid marking the new error message for translation as it > + will be done in the next commit when the message is generalized to other > + special refs. Looking at the change below, the new message is in fact marked for translation. I don't think this matters (other than the commit message being confusing) as the translators will only see the final version of the massage. > Helped-by: Junio C Hamano <gitster@pobox.com> > Signed-off-by: Michael Lohmann <mi.al.lohmann@gmail.com> > Signed-off-by: Junio C Hamano <gitster@pobox.com> > @@ revision.c: static void prepare_show_merge(struct rev_info *revs) > + &oid, NULL)) > die("--merge without MERGE_HEAD?"); > + if (is_null_oid(&oid)) > -+ die(_("MERGE_HEAD is a symbolic ref?")); > ++ die(_("MERGE_HEAD exists but is a symbolic ref")); The new message is marked with _(...) so will be translated. > other = lookup_commit_or_die(&oid, "MERGE_HEAD"); > add_pending_object(revs, &head->object, "HEAD"); > add_pending_object(revs, &other->object, "MERGE_HEAD"); > 2: 1641c4be81 ! 2: 749abadc04 revision: implement `git log --merge` also for rebase/cherry-pick/revert > @@ revision.c: static void add_pending_commit_list(struct rev_info *revs, > + RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, > + oid, NULL)) { > + if (is_null_oid(oid)) > -+ die(_("%s is a symbolic ref?"), other_head[i]); > ++ die(_("%s exists but is a symbolic ref"), other_head[i]); The second patch updates the message and this new version retains the _(...) Best Wishes Phillip > + return other_head[i]; > + } > + > @@ revision.c: static void prepare_show_merge(struct rev_info *revs) > - &oid, NULL)) > - die("--merge without MERGE_HEAD?"); > - if (is_null_oid(&oid)) > -- die(_("MERGE_HEAD is a symbolic ref?")); > +- die(_("MERGE_HEAD exists but is a symbolic ref")); > - other = lookup_commit_or_die(&oid, "MERGE_HEAD"); > + other_name = lookup_other_head(&oid); > + other = lookup_commit_or_die(&oid, other_name); >
Hi Phillip, Le 2024-02-28 à 09:40, phillip.wood123@gmail.com a écrit : > Hi Philippe > > On 28/02/2024 13:54, Philippe Blain wrote: >> Range-diff versus v5: >> >> 1: c9536431d1 ! 1: 363657561c revision: ensure MERGE_HEAD is a ref in prepare_show_merge >> @@ Commit message >> (2) obtain the oid without any prefixing by refs.c:repo_dwim_ref() >> (3) error out when MERGE_HEAD is a symref. >> + Note that we avoid marking the new error message for translation as it >> + will be done in the next commit when the message is generalized to other >> + special refs. > > Looking at the change below, the new message is in fact marked for translation. I don't think this matters (other than the commit message being confusing) as the translators will only see the final version of the massage. Oops, you are right, I wanted to do what I wrote but ended up marking it also in 1/2. Junio already merged it to next, so I guess it's gonna stay that way. Thanks for your review, Philippe.
Changes in v6: - Changed the error message added in 1/2 and adjusted in 2/2 to avoid a rhetorical question Changes in v5: - Marked error messages for translation and tweaked them as suggested by Phillip - Reworded the message of 2/2 as suggested by Phillip - Removed the change to gitk's doc in 2/2 as pointed out by Johannes - Fixed the trailers in 2/2 - Improved the doc in 2/2 as suggested by Phillip and Jean-Noël Changes in v4: - Added a commit message for 2/2 detailing the use case and summarizing the discussion in the thread - Adjusted the documentation of the option --- Michael Lohmann (2): revision: ensure MERGE_HEAD is a ref in prepare_show_merge revision: implement `git log --merge` also for rebase/cherry-pick/revert Documentation/rev-list-options.txt | 7 +++++-- revision.c | 27 +++++++++++++++++++++++---- 2 files changed, 28 insertions(+), 6 deletions(-) --- base-commit: 3c2a3fdc388747b9eaf4a4a4f2035c1c9ddb26d0 change-id: 20240210-ml-log-merge-with-cherry-pick-and-other-pseudo-heads-05bd8e8797db Range-diff versus v5: 1: c9536431d1 ! 1: 363657561c revision: ensure MERGE_HEAD is a ref in prepare_show_merge @@ Commit message (2) obtain the oid without any prefixing by refs.c:repo_dwim_ref() (3) error out when MERGE_HEAD is a symref. + Note that we avoid marking the new error message for translation as it + will be done in the next commit when the message is generalized to other + special refs. + Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Michael Lohmann <mi.al.lohmann@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> @@ revision.c: static void prepare_show_merge(struct rev_info *revs) + &oid, NULL)) die("--merge without MERGE_HEAD?"); + if (is_null_oid(&oid)) -+ die(_("MERGE_HEAD is a symbolic ref?")); ++ die(_("MERGE_HEAD exists but is a symbolic ref")); other = lookup_commit_or_die(&oid, "MERGE_HEAD"); add_pending_object(revs, &head->object, "HEAD"); add_pending_object(revs, &other->object, "MERGE_HEAD"); 2: 1641c4be81 ! 2: 749abadc04 revision: implement `git log --merge` also for rebase/cherry-pick/revert @@ revision.c: static void add_pending_commit_list(struct rev_info *revs, + RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, + oid, NULL)) { + if (is_null_oid(oid)) -+ die(_("%s is a symbolic ref?"), other_head[i]); ++ die(_("%s exists but is a symbolic ref"), other_head[i]); + return other_head[i]; + } + @@ revision.c: static void prepare_show_merge(struct rev_info *revs) - &oid, NULL)) - die("--merge without MERGE_HEAD?"); - if (is_null_oid(&oid)) -- die(_("MERGE_HEAD is a symbolic ref?")); +- die(_("MERGE_HEAD exists but is a symbolic ref")); - other = lookup_commit_or_die(&oid, "MERGE_HEAD"); + other_name = lookup_other_head(&oid); + other = lookup_commit_or_die(&oid, other_name);