Re: [PATCH 5/5] repo-settings: create feature.experimental setting

From: Derrick Stolee <stolee@gmail.com>
Date: Jul 25, 2019, 01:47 UTC
Message-ID: <b9843571-5358-a33e-40cf-0d6d30d30766@gmail.com>
In-Reply-To: <nycvar.QRO.7.76.6.1907231708280.21907@tvgsbejvaqbjf.bet>

On 7/23/2019 11:20 AM, Johannes Schindelin wrote:

Show 23 quoted lines

> Hi Stolee,
> 
> On Mon, 22 Jul 2019, Derrick Stolee via GitGitGadget wrote:
>>  struct path_hashmap_entry {
>>  	struct hashmap_entry e;
>> @@ -1375,10 +1376,14 @@ static int handle_rename_via_dir(struct merge_options *opt,
>>  	 * there is no content merge to do; just move the file into the
>>  	 * desired final location.
>>  	 */
>> +	struct repository *r = the_repository;
>>  	const struct rename *ren = ci->ren1;
>>  	const struct diff_filespec *dest = ren->pair->two;
>>  	char *file_path = dest->path;
>> -	int mark_conflicted = (opt->detect_directory_renames == 1);
> 
> I actually don't think that we want to do that; the `opt` parameter is
> passed to the merge recursive algorithm specifically so that it can be
> overridden by the caller.
> 
> Instead, what we should do, I think, is to change `init_merge_options()`
> (which already gets a parameter of type `struct repository *`) so that
> it does not hard-code the `detect_directory_renames` value to `1` but to
> query the repo settings instead.

[snip]

Show 7 quoted lines

>> @@ -3687,7 +3690,6 @@ void init_merge_options(struct merge_options *opt,
>>  	opt->renormalize = 0;
>>  	opt->diff_detect_rename = -1;
>>  	opt->merge_detect_rename = -1;
>> -	opt->detect_directory_renames = 1;
> 
> In other words: here.

Yes, thanks! Your suggestion makes much more sense and makes the diff much simpler. No reason to couple too closely to the config storage here.

I think you gave me enough feedback to merit a v2 before others chime in, so I'll send one after I validate my current version.

Thanks, -Stolee