Re: [PATCH] merge-recursive: option to disable renames

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



"Felipe Gonçalves Assis"  <felipeg.assis@xxxxxxxxx> writes:

> +no-renames;;
> +	Turn off rename detection.
> +	See also linkgit:git-diff[1] `--no-renames`.

Even though by default for merge-recursive the rename detection is
on, if we are adding an option to control this aspect of the
behaviour from the command line, it should follow the usual pattern,
i.e.

 (1) the code to parse options would allow an earlier "--no-renames"
     on the command line to be overridden with a later "--renames"; and

 (2) the description in the documentation would be headed by
     "--[no-]renames", describes which one is the default, etc.

> diff --git a/merge-recursive.c b/merge-recursive.c
> index 8eabde2..ca67805 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1839,9 +1839,16 @@ int merge_trees(struct merge_options *o,
>  
>  		entries = get_unmerged();
>  		record_df_conflict_files(o, entries);
> -		re_head  = get_renames(o, head, common, head, merge, entries);
> -		re_merge = get_renames(o, merge, common, head, merge, entries);
> -		clean = process_renames(o, re_head, re_merge);
> +		if (o->detect_rename) {
> +			re_head  = get_renames(o, head, common, head, merge, entries);
> +			re_merge = get_renames(o, merge, common, head, merge, entries);
> +			clean = process_renames(o, re_head, re_merge);
> +		}
> +		else {
> +			re_head  = xcalloc(1, sizeof(struct string_list));
> +			re_merge = xcalloc(1, sizeof(struct string_list));
> +			clean = 1;
> +		}

Yup, this is much nicer than butchering diffcore-rename.c for no
good reason ;-).

Even if you _know_ that process_renames() currently does not do
anything other than returning 1, it is a bad idea to hardcode that
knowledge on the caller's side.  Perhaps

>  		entries = get_unmerged();
>  		record_df_conflict_files(o, entries);
> -		re_head  = get_renames(o, head, common, head, merge, entries);
> -		re_merge = get_renames(o, merge, common, head, merge, entries);
> +		if (o->detect_rename) {
> +			re_head  = get_renames(o, head, common, head, merge, entries);
> +			re_merge = get_renames(o, merge, common, head, merge, entries);
> +		} else {
> +			re_head  = xcalloc(1, sizeof(struct string_list));
> +			re_merge = xcalloc(1, sizeof(struct string_list));
> +		}
> 		clean = process_renames(o, re_head, re_merge);

Preparing a new empty string_list instance with xcalloc() without
doing string_list_init() is probably "caller knows too much about
implementation detail of the API", but get_renames() seems to do so
already, so I'll let it pass.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]