diff mbox series

[1/4] merge: simplify initialization

Message ID 20210613225836.1009569-2-felipe.contreras@gmail.com (mailing list archive)
State New
Headers show
Series merge: cleanups and fix | expand

Commit Message

Felipe Contreras June 13, 2021, 10:58 p.m. UTC
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/merge-file.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

Ævar Arnfjörð Bjarmason June 15, 2021, 7:27 p.m. UTC | #1
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?
Felipe Contreras June 17, 2021, 10:35 p.m. UTC | #2
Æ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 mbox series

Patch

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);