On 2021.08.10 15:33, Junio C Hamano wrote:
> Josh Steadmon <steadmon@xxxxxxxxxx> writes:
>
> > + [ADVICE_SKIPPED_CHERRY_PICKS] = { "skippedCherryPicks", 1},
>
> No need to resend to only fix this, but I'll add SP after "1" to
> match surrounding lines while queuing the patch.
Ack. Fixed in V3. Do you also want me to rebase this on
ab/retire-advice-config?
> > @@ -5149,8 +5150,13 @@ static int make_script_with_merges(struct pretty_print_context *pp,
> > oidset_insert(&interesting, &commit->object.oid);
> >
> > is_empty = is_original_commit_empty(commit);
> > - if (!is_empty && (commit->object.flags & PATCHSAME))
> > + if (!is_empty && (commit->object.flags & PATCHSAME)) {
> > + advise_if_enabled(ADVICE_SKIPPED_CHERRY_PICKS,
> > + _("skipped previously applied commit %s"),
> > + short_commit_name(commit));
> > + skipped_commit = 1;
> > continue;
> > + }
> > if (is_empty && !keep_empty)
> > continue;
> >
> > @@ -5214,6 +5220,9 @@ static int make_script_with_merges(struct pretty_print_context *pp,
> > oidcpy(&entry->entry.oid, &commit->object.oid);
> > oidmap_put(&commit2todo, entry);
> > }
> > + if (skipped_commit)
> > + advise_if_enabled(ADVICE_SKIPPED_CHERRY_PICKS,
> > + _("use --reapply-cherry-picks to include skipped commits"));
>
> I agree with the change in this hunk that advanced users may want to
> squelch this "what to do" hint.
>
> I am not sure about the earlier hunk that reports when some commits
> have actually been skipped. When --no-reapply-cherry-picks is in
> effect, the user is expecting that some commits are cherry-picks
> among other (hopefully the majority of) commits, and even those
> users who do not want to be taught how to use the command would want
> to learn the fact that some commits were skipped (and which ones).
>
> Using two separate advice configuration variables feel way overkill
> for this. I wonder if the previous hunk should use warning(), i.e.
Switched these to warnings, squelched by "--quiet" as suggested below.
> > + if (!is_empty && (commit->object.flags & PATCHSAME)) {
> > + warning(_("skipped previously applied commit %s"),
> > + short_commit_name(commit));
> > + skipped_commit = 1;
> > continue;
> > + }
>
> possibly squelched by "git rebase --quiet".
>
