| Message ID | 20210613225836.1009569-2-felipe.contreras@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | merge: cleanups and fix | expand |
On Sun, Jun 13 2021, Felipe Contreras wrote: > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> > --- > 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? Also, not a problem in your commit, but we check for that constant in only one place, as: XDL_MERGE_ZEALOUS < level 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?
Ævar Arnfjörð Bjarmason wrote: > > On Sun, Jun 13 2021, Felipe Contreras wrote: > > > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> > > --- > > 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/
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);
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> --- builtin/merge-file.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)