From: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Date: Fri, 25 Sep 2026 17:51:08 GMT
Subject: Re: [RFC PATCH 2/3] setup: introduce new helper 'is_git_directory_verbose'
Message-ID: <c7808d4b-36f8-4583-8836-7b7d8bc24905@gmail.com>
In-Reply-To: <xmqqcxu2z4cy.fsf@gitster.g>

On 9/25/26 03:41, Junio C Hamano wrote:
> Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:
> 
>> diff --git a/setup.c b/setup.c
>> index 0d157ac254..b3b53a1cfc 100644
>>   
>>   	/* Make sure it is a "refs/.." symlink */
>>   	if (S_ISLNK(st.st_mode)) {
>>   		len = readlink(path, buffer, sizeof(buffer)-1);
>>   		if (len >= 5 && !memcmp("refs/", buffer, 5))
>>   			return 0;
>> +		if (len == -1 && err)
>> +			strbuf_addf(
>> +				err,
>> +				_("could not read the symlink HEAD at '%s'"),
>> +				path
>> +			);
>> +		else if (err)
>> +			strbuf_addf(
>> +				err,
>> +				_("HEAD is a symlink ('%s') but target"
>> +				  " lives outside refs/"),
>> +				path
>> +			);
>>   		return -1;
>>   	}
> 
> All of the above (and below---ellided) look fairly funny way to
> indent them.  If you are trying ot match the style used in the
> existing code around the same area, I wouldn't complain, but I
> didn't look beyond what is visible in the patch.
> 

Indeed. My bad. Does the following look like a good indentation style 
for shorter messages?

	if (lstat(path, &st) < 0) {
		if (err)
			strbuf_addf(err, _("could not stat HEAD at '%s'"), path);
		return -1;
          }

... and this for messages that are a bit longer:

	if (len == -1 && err)
		strbuf_addf(err, _("could not read the symlink HEAD at '%s'"),
			    path);
	else if (err)
		strbuf_addf(err, _("HEAD is a symlink ('%s') but target lives"
				   " outside refs/"), path);


-- 
Sivaraam

PS: I'm not yet very sure if my MUA will send this as intended. Will 
resend if it doesn't. Excuse the noise in advance.