Linux kbuild/kconfig development
 help / color / mirror / Atom feed
From: Julian Braha <julianbraha@gmail.com>
To: sashiko-reviews@lists.linux.dev, nathan@kernel.org, nsc@kernel.org
Cc: ojeda@kernel.org, a.hindborg@kernel.org, nico@fluxnic.net,
	rdunlap@infradead.org, grahamr@qti.qualcomm.com,
	jakub.git@horky.net, linux-kbuild@vger.kernel.org,
	gary@garyguo.net
Subject: Re: [PATCH v2 2/3] kconfig: wrap long expressions in menuconfig
Date: Wed, 11 Nov 2026 18:07:55 +0000	[thread overview]
Message-ID: <86085222-625d-41aa-8b60-2ac8960ffb76@gmail.com> (raw)
In-Reply-To: <sashiko-outbox-209420@kernel.org>


[-- Attachment #1.1: Type: text/plain, Size: 3688 bytes --]

On 11/11/26 17:35, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Low] kconfig: broken '(' and operator wrapping in
>   expr_print_gstr_helper()
> --
> 
> commit 89202a55a9cf081fc680fbfa051ebe8d0cfa7113
> Author: Julian Braha <julianbraha@gmail.com>
> 
> kconfig: wrap long expressions in menuconfig
> 
> This commit wraps long Kconfig expressions across lines with matching
> indentation in menuconfig search and help text, keeping '!' prefixes and
> value annotations attached to their symbols.
> 
>> diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
>> index 2b91d16bf14fb..9284ccb20259c 100644
>> --- a/scripts/kconfig/expr.c
>> +++ b/scripts/kconfig/expr.c
>> @@ -1155,30 +1155,40 @@ static void expr_print_gstr_helper(void *data, struct symbol *sym, const char *s
>>  	if (sym)
>>  		sym_str = sym_get_string_value(sym);
>>  
>> -	if (gs->max_width) {
>> +	if (gs->max_width && !strchr(str, '\n')) {
>>  		unsigned extra_length = strlen(str);
>>  		const char *last_cr = strrchr(gs->s, '\n');
>>  		unsigned last_line_length;
>> +		unsigned int prefix_len = strlen(gs->s) - gs->wrap_pos;
>>  
>> -		if (sym_str)
>> +		/* Ensure the value annotation stays attached to the symbol on wrap */
>> +		if (sym_str && sym->type != S_UNKNOWN)
>>  			extra_length += 4 + strlen(sym_str);
>>  
>> -		if (!last_cr)
>> -			last_cr = gs->s;
>> +		last_line_length = strlen(last_cr ? last_cr + 1 : gs->s);
>>  
>> -		last_line_length = strlen(gs->s) - (last_cr - gs->s);
>> -
>> -		if ((last_line_length + extra_length) > gs->max_width)
>> -			str_append(gs, "\\\n");
>> +		if (last_line_length > gs->indent + prefix_len &&
>> +		    last_line_length + extra_length > gs->max_width) {
>> +			gs->s[gs->wrap_pos] = '\0';
>> +			str_printf(gs, "\n%*s", gs->indent, "");
>> +			while (prefix_len--)
>> +				str_append(gs, "!");
>> +			if (!sym)
>> +				str += strspn(str, " ");
>> +		}
>>  	}
>>  
>>  	str_append(gs, str);
>>  	if (sym && sym->type != S_UNKNOWN)
>>  		str_printf(gs, " [=%s]", sym_str);
>> +	/* Ensure '!' stays attached to the symbol on wrap */
>> +	if (sym || strcmp(str, "!"))
>> +		gs->wrap_pos = strlen(gs->s);
>>  }
> [Severity: Low]
> Does expr_print_gstr_helper() mishandle wrapping around "(" and wrapped
> operators?
> 
> When expr_print() formats a negated parenthesized expression such as
> "!(A || B)", it emits "!" and then "(" before the first symbol.  Because
> strcmp("(", "!") is non-zero, printing "(" updates gs->wrap_pos to point
> after "!(" and resets prefix_len to 0.  If the first symbol plus its
> "[=...]" annotation then exceeds gs->max_width, it wraps after "(",
> leaving "!(" (or "(") dangling at the end of the previous line.
> 
> In addition, the guard "last_line_length > gs->indent + prefix_len"
> assumes only indentation and "!" can precede the first symbol on a line.
> After " && " or " || " wraps onto a new line (or after "(" or "[" is
> printed at the start of a line), gs->wrap_pos is updated past that token,
> making last_line_length greater than gs->indent + prefix_len before any
> symbol is printed on the line.  If the following symbol and its value
> annotation exceed gs->max_width, expr_print_gstr_helper() wraps a second
> time and leaves "&& ", "|| ", "(" or "[" stranded on a line by itself.
> 
> -- Sashiko AI review · https://sashiko.dev/#/
> patchset/20261110173526.1548434-1-julianbraha@gmail.com?part=2
> 

Sashiko is hallucinating this time.

I mean, what it's saying is correct, but this is intended behavior not a
bug.

- Julian Braha

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

  reply	other threads:[~2026-11-11 18:07 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-10 17:35 [PATCH v2 0/3] kconfig: wrap long expressions in menuconfig Julian Braha
2026-11-10 17:35 ` [PATCH v2 1/3] kconfig: print the output of expr_alloc_and() instead of printing '&&' Julian Braha
2026-11-10 17:35 ` [PATCH v2 2/3] kconfig: wrap long expressions in menuconfig Julian Braha
2026-11-11 17:35   ` sashiko-bot
2026-11-11 18:07     ` Julian Braha [this message]
2026-11-10 17:35 ` [PATCH v2 3/3] kconfig: show continuation at the end of wrapped lines Julian Braha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=86085222-625d-41aa-8b60-2ac8960ffb76@gmail.com \
    --to=julianbraha@gmail.com \
    --cc=a.hindborg@kernel.org \
    --cc=gary@garyguo.net \
    --cc=grahamr@qti.qualcomm.com \
    --cc=jakub.git@horky.net \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=nathan@kernel.org \
    --cc=nico@fluxnic.net \
    --cc=nsc@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox