Re: [PATCH] Fix pager search behavior with REG_NEWLINE.

"Kevin J. McCarthy" <[email protected]> Thu, 21 May 2026 18:20:04 +0800
Newsgroups	gmane.mail.mutt.devel
Message-ID	<ag7cVLee4v87oa2W@qinghai>
On Thu, May 21, 2026 at 11:24:38AM +0200, Alejandro Colomar via Mutt-dev wrote:
>>
>>  pager.c | 69 ++++++++++++++++++++++++++++++++++++---------------------
>>  1 file changed, 44 insertions(+), 25 deletions(-)

Hi Alex,

Thanks for the review.

I should have mentioned that this patch is basically stitching together 
two parts of the existing code.  Both of them are old, and so are... to 
say it politely... "compact".

The search chunk was relocated into a function resolve_search().

The newline removal/re-add was copied from a different function in the
pager code, match_body_patterns().

>> diff --git a/pager.c b/pager.c
>> index 92fa152a..75221643 100644
>> --- a/pager.c
>> +++ b/pager.c
>> @@ -1268,6 +1268,47 @@ bail:
>>    return rc;
>>  }
>>
>> +static void resolve_search(struct line_t *lineInfo, int n, char *fmt,
>> +                           regex_t *SearchRE)
>> +{
>> +  size_t fmtlen;
>> +  int has_nl = 0, offset = 0;
>> +  regmatch_t pmatch[1];
>> +
>> +  /* don't consider line endings part of the buffer
>> +   * for regex matching */
>> +  if ((fmtlen = mutt_strlen(fmt)) > 0 && fmt[fmtlen - 1] == '\n')
>
>I would very much rewrite the above in two lines.  I don't see a good
>reason to embed the assignment in the conditional (and that often leads
>to bugs due to accidents).  I suggest:
>
>	fmtlen = mutt_strlen(fmt);
>	if (fmtlen > 0 && fmt[fmtlen - 1] == '\n')

Sure I'll make this change.  Again, in my defense, this is verbatim
pulled from match_body_patterns().  So although it's compact, I knew it
was working.

>> +  {
>> +    has_nl = 1;
>> +    fmt[fmtlen - 1] = 0;
>> +  }
>> +
>> +  offset = 0;
>> +  lineInfo[n].search_cnt = 0;
>> +  while (regexec(SearchRE, (char *) fmt + offset, 1, pmatch, (offset ? REG_NOTBOL : 0)) == 0)
>
>What's the point of the cast?  fmt is already char*, right?

Actually, no.  buf and fmt are unsigned char *.  In parts of the pager
code, the chars are upcast to int, so I'm assuming this was done
purposely to avoid high-bit set signed char conversion issues in those
places.

[snipping out your comments after realizing they were for relocated
code, since I'm not up for rewriting pager code right now. :-)]

>Have a lovely day!
>Alex

Thanks, you too.

-- 
Kevin J. McCarthy
GPG Fingerprint: 8975 A9B3 3AA3 7910 385C  5308 ADEF 7684 8031 6BDA
signature.asc (application/pgp-signature, 833 B)