Ævar Arnfjörð Bjarmason wrote:
>
> On Sun, Jun 13 2021, Felipe Contreras wrote:
>
> > Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
> > ---
> > builtin/merge-file.c | 6 +-----
> > 1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/builtin/merge-file.c b/builtin/merge-file.c
> > index 06a2f90c48..0186f4156a 100644
> > --- a/builtin/merge-file.c
> > +++ b/builtin/merge-file.c
> > @@ -28,7 +28,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
> > const char *names[3] = { NULL, NULL, NULL };
> > mmfile_t mmfs[3];
> > mmbuffer_t result = {NULL, 0};
> > - xmparam_t xmp = {{0}};
> > + xmparam_t xmp = { .level = XDL_MERGE_ZEALOUS_ALNUM };
> > int ret = 0, i = 0, to_stdout = 0;
> > int quiet = 0;
> > struct option options[] = {
> > @@ -48,10 +48,6 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
> > OPT_END(),
> > };
> >
> > - xmp.level = XDL_MERGE_ZEALOUS_ALNUM;
> > - xmp.style = 0;
> > - xmp.favor = 0;
> > -
> > if (startup_info->have_repository) {
> > /* Read the configuration file */
> > git_config(git_xmerge_config, NULL);
>
> Looks good, maybe we can fix the similar code in ll_xdl_merge() while
> we're at it?
Yeah, but since code cleanups for the sake of cleaning up are frowned
upon [1] it's not clear how we would get these merged in, since Elijah
has gunned down the obvious fix in patch 4.
Now the cleanups are orphaned.
> Also, not a problem in your commit, but we check for that constant in
> only one place, as:
>
> XDL_MERGE_ZEALOUS < level
My mind can't process that, so...
level > XDL_MERGE_ZEALOUS
> Urgh, do you know if there's some reason we're not doing level ==
> XDL_MERGE_ZEALOUS_ALNUM, or at least level >= XDL_MERGE_ZEALOUS_ALNUM
> there?
Nope. To the best of my knowledge that's what the code actually meant.
In general the code in xdiff/* seems to be following a very different
style than the rest of git's core.
Cheers.
[1] https://lore.kernel.org/git/xmqqbl87zyra.fsf@gitster.g/
--
Felipe Contreras
