Re: [PATCH] xdiff: improve trimming preprocessing

[Jun Wu <quark@fb.com>]

Excerpts from Eric Sunshine's message of 2018-03-06 14:23:46 -0500:
> On Tue, Mar 6, 2018 at 6:53 AM, Jun Wu <quark@fb.com> wrote:
> > +  printf "x%.0s" {1..934} >>d # pad common suffix to 1024 bytes
> 
> The expression {x..y} is not portable to non-POSIX shells.

Is there a recommended way to generate a repetitive string?
Maybe `seq 1000 | sed 's/.*/x/'` ?

> > +    fgrep "@@ -1001 +1000,0 @@"
> > +'
> 
> Style: Mix of tabs and spaces for indentation. Please indent only with
> tabs throughout the patch.

Sorry. Will fix in V2.
 
> > +       /* prefix - need line count for xecfg->ptrimmed */
> > +       for (i = 0; ++i < smaller && *ap == *bp;) {
> > +               lines += (*ap == '\n');
> > +               ap++, bp++;
> 
> Is there a good reason for not placing 'ap++, bp++' directly in the 'for'?

"lines += (*ap == '\n');" needs the "ap" before adding. Alternatives are

    for (i = 0; ++i < smaller && *ap == *bp; ) /* 1 */
        lines += (*ap++, *bp++) == '\n';

    for (i = 0; ++i < smaller && *ap == *bp; ap++, bp++) /* 2 */
        lines += (*(ap - 1) == '\n');

Maybe will pick /* 1 */ to make the code shorter.