| Message ID | c2a2be336e0ed7966b6ab0ef004f150537167b55.1741362522.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Small new merge-ort features, prepping for deletion of merge-recursive.[ch] | expand |
On Fri, Mar 07, 2025 at 03:48:42PM +0000, Elijah Newren via GitGitGadget wrote: > From: Elijah Newren <newren@gmail.com> > > Various callers such as am & checkout set the merge verbosity to 0 to > avoid having conflict messages printed. While this could be achieved by > avoiding the wrappers from merge-ort-wrappers and instead passing 0 for > display_update_msgs to merge_switch_to_result(), for simplicity of > converting callers simply allow them to also achieve this with the > merge-ort-wrappers by setting verbosity to 0. > > Signed-off-by: Elijah Newren <newren@gmail.com> > --- > merge-ort.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/merge-ort.c b/merge-ort.c > index a6960b6a1b4..8021083c112 100644 > --- a/merge-ort.c > +++ b/merge-ort.c > @@ -799,6 +799,8 @@ static void path_msg(struct merge_options *opt, > return; /* Do not record mere hints in headers */ > if (opt->priv->call_depth && opt->verbosity < 5) > return; /* Ignore messages from inner merges */ > + if (!opt->verbosity) > + return; Looks trivially correct ;-). Should we add a test to ensure that we don't regress this behavior in the future? Thanks, Taylor
On Wed, Mar 12, 2025 at 1:03 PM Taylor Blau <me@ttaylorr.com> wrote: > > On Fri, Mar 07, 2025 at 03:48:42PM +0000, Elijah Newren via GitGitGadget wrote: > > From: Elijah Newren <newren@gmail.com> > > > > Various callers such as am & checkout set the merge verbosity to 0 to > > avoid having conflict messages printed. While this could be achieved by > > avoiding the wrappers from merge-ort-wrappers and instead passing 0 for > > display_update_msgs to merge_switch_to_result(), for simplicity of > > converting callers simply allow them to also achieve this with the > > merge-ort-wrappers by setting verbosity to 0. > > > > Signed-off-by: Elijah Newren <newren@gmail.com> > > --- > > merge-ort.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/merge-ort.c b/merge-ort.c > > index a6960b6a1b4..8021083c112 100644 > > --- a/merge-ort.c > > +++ b/merge-ort.c > > @@ -799,6 +799,8 @@ static void path_msg(struct merge_options *opt, > > return; /* Do not record mere hints in headers */ > > if (opt->priv->call_depth && opt->verbosity < 5) > > return; /* Ignore messages from inner merges */ > > + if (!opt->verbosity) > > + return; > > Looks trivially correct ;-). Yeah, but after thinking about it more I don't like it here. I don't really like the opt->verbosity thing from merge-recursive, it simply survived because it was a semi-public API, but I'd rather minimize it. So, in the next round I think I'm going to stick this in the merge-ort-wrappers rather than have it appear here. > Should we add a test to ensure that we don't regress this behavior in the future? I'd rather reuse `git {switch,checkout} -m`'s tests for this purpose rather than adding new ones (and perhaps the ones from git-am; can't remember if those also caught this). If you feel strongly about this, I'll just squash this into the later patch and make it bigger so we don't need more redundant tests.
On Wed, Mar 12, 2025 at 02:44:24PM -0700, Elijah Newren wrote: > > Should we add a test to ensure that we don't regress this behavior in the future? > > I'd rather reuse `git {switch,checkout} -m`'s tests for this purpose > rather than adding new ones (and perhaps the ones from git-am; can't > remember if those also caught this). If you feel strongly about this, > I'll just squash this into the later patch and make it bigger so we > don't need more redundant tests. No, I don't feel strongly. Thanks, Taylor
diff --git a/merge-ort.c b/merge-ort.c index a6960b6a1b4..8021083c112 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -799,6 +799,8 @@ static void path_msg(struct merge_options *opt, return; /* Do not record mere hints in headers */ if (opt->priv->call_depth && opt->verbosity < 5) return; /* Ignore messages from inner merges */ + if (!opt->verbosity) + return; /* Ensure path_conflicts (ptr to array of logical_conflict) allocated */ path_conflicts = strmap_get(&opt->priv->conflicts, primary_path);