From: Phillip Wood <phillip.wood123@gmail.com>
Date: Tue, 01 Aug 2023 18:49:34 GMT
Subject: Re: [PATCH v3 3/7] sequencer: use rebase_path_message()
Message-ID: <619e458b-218b-a790-dfb4-9200e201b513@gmail.com>
In-Reply-To: <xmqq4jlid6cx.fsf@gitster.g>

On 01/08/2023 18:23, Junio C Hamano wrote:
> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> Rather than constructing the path in a struct strbuf use the ready
>> made function to get the path name instead. This was the last
>> remaining use of the strbuf so remove it as well.
> 
> The same comment about "get_dir() vs hardcoded rebase-merge" applies
> here, I think.  And the same (1) assertion to ensure that we are in
> "rebase -i" when make_patch() is called should give us a sufficient
> safety valve,

Agreed

> or (2) instead of hardcoding rebase_path_message(),
> call it sequencer_path_message() and switch at runtime behaving the
> same way as get_dir(opts) based version, would also work.

I think that would me misleading because cherry-pick/revert do not 
create that file - they rely on "git commit" reading .git/MERGE_MSG

Best Wishes

Phillip

> Thanks.
> 
>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>> ---
>>   sequencer.c | 7 ++-----
>>   1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/sequencer.c b/sequencer.c
>> index 70b0a7023b0..dbddd19b2c2 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -3501,7 +3501,6 @@ static int make_patch(struct repository *r,
>>   		      struct commit *commit,
>>   		      struct replay_opts *opts)
>>   {
>> -	struct strbuf buf = STRBUF_INIT;
>>   	struct rev_info log_tree_opt;
>>   	const char *subject;
>>   	char hex[GIT_MAX_HEXSZ + 1];
>> @@ -3529,18 +3528,16 @@ static int make_patch(struct repository *r,
>>   		fclose(log_tree_opt.diffopt.file);
>>   	}
>>   
>> -	strbuf_addf(&buf, "%s/message", get_dir(opts));
>> -	if (!file_exists(buf.buf)) {
>> +	if (!file_exists(rebase_path_message())) {
>>   		const char *encoding = get_commit_output_encoding();
>>   		const char *commit_buffer = repo_logmsg_reencode(r,
>>   								 commit, NULL,
>>   								 encoding);
>>   		find_commit_subject(commit_buffer, &subject);
>> -		res |= write_message(subject, strlen(subject), buf.buf, 1);
>> +		res |= write_message(subject, strlen(subject), rebase_path_message(), 1);
>>   		repo_unuse_commit_buffer(r, commit,
>>   					 commit_buffer);
>>   	}
>> -	strbuf_release(&buf);
>>   	release_revisions(&log_tree_opt);
>>   
>>   	return res;