Re: [PATCH v12 1/4] stash: add --label-ours, --label-theirs, --label-base for apply

[Junio C Hamano <gitster@pobox.com>]

Phillip Wood <phillip.wood123@gmail.com> writes:

>> +static int do_apply_stash_with_labels(const char *prefix,
>> +				      struct stash_info *info,
>> +				      int index, int quiet,
>> +				      const char *label_ours, const char *label_theirs,
>> +				      const char *label_base)
>
> There are only four callers of do_apply_stash so it might be better just 
> to change the function signature and update the existing callers rather 
> than adding another function.
>
>> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
>> index 70879941c2..00bcb1f802 100755
>> --- a/t/t3903-stash.sh
>> +++ b/t/t3903-stash.sh
>> @@ -1666,6 +1666,35 @@ test_expect_success 'restore untracked files even when we hit conflicts' '
>>   	)
>>   '
>>   
>> +test_expect_success 'apply with custom conflict labels' '
>> +	git init conflict_labels &&
>> +	(
>
> I'm still unclear why we're creating a new repository here. Our test 
> suite is slow enough already without each test spending time creating 
> its own repository. There doesn't seem to be anything here that requires 
> isolating the test in this way.

Both are exellent points.  

I also agree with your comments on create_autostash_ref() in [2/4],
extending apply_autostash_ref() with optional three or four extra
parameters and updating existing callers in [3/4].

I have v10 already merged to 'next', but I think it is better to
revert the merge and give these finishing touches, as we are not
in a rush to add more topics to 'next' before 2.54 final anyway.

Thanks.