Re: [PATCH v4 05/12] builtin: add new "history" command

From: Karthik Nayak <karthik.188@gmail.com>
Date: Oct 22, 2025, 12:12 UTC
Message-ID: <CAOLa=ZTHxw8uCbo=oHq=LF=qX=sufnWKJTC6F2YLsrZ8EyxsYw@mail.gmail.com>
In-Reply-To: <xmqqldl31uhq.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 23 quoted lines

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>>> +	const char * const usage[] = {
>>> +		N_("git history [<options>]"),
>>> +		NULL,
>>> +	};
>>
>> Nit: We have pointer alignment set to 'Right' in our styling guide and
>> also mentioned in our 'Documentation/CodingGuidelines'
>>
>>    When declaring pointers, the star sides with the variable
>>    name, i.e. "char *string", not "char* string" or
>>    "char * string".  This makes it easier to understand code
>>    like "char *string, c;".
>
> But there is nothing specified for an asterisk that cannot side with
> variable name, like the one we see above.  I _think_ the "space on
> both sides" is the prevalent style, but I do not know (although I
> suspect you do---as the person with most changes in it) what (y)our
> clang format configuration wants to do.  Can you make sure the tool
> suggests the style that matches the prevailing style?
>
> Thanks.

I looked into this, and unfortunately it [1] doesn't support such granularity.

So for something like `const char * const usage`, it only cares about the alignment of the pointer with respect to the tokens surrounding it.

With our current setting of `PointerAlignment: Right`, this means it would expect to have `const char *const usage` which is not the prevalent style.

[1]: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#pointeralignment