Re: [PATCH] gpg-interface: trim only CR characters that precede LF

>
> Okhuomon Ajayi <okhuomonajayi54@gmail.com> writes:
>
> >  /*
> > - * Strip CR from the line endings, in case we are on Windows.
> > - * NEEDSWORK: make it trim only CRs before LFs and rename
> > + * Trim CR characters only when they appear before LF (\r\n) line endings.
> > + * This avoids removing legitimate lone CRs from teh content.
>
> "teh" -> "the".  I know, I myself often make teh same typo.
>
> >   */
> > -static void remove_cr_after(struct strbuf *buffer, size_t offset)
> > +static void trim_cr_before_lf(struct strbuf *buffer, size_t offset)
>
> In other words, this normalizes crlf to lf line ending.
>
> >  {
> >       size_t i, j;
> >
> >       for (i = j = offset; i < buffer->len; i++) {
> > -             if (buffer->buf[i] != '\r') {
> > +          /* skip CR only if it comes right before LF */
> > +             if (buffer->buf[i] == '\r' && i + 1 < buffer->len && buffer->buf[i+1] == '\n')
>
> Are two different mixture of tabs and spaces used in the above two
> lines?  I think they wanted to begin at the same column.
>
> Also, the second line is overly long that it does not even fit on my
> 92-column wide terminal (yes, 80 is the limit, but this will let a
> line in the patches quoted a few times to still fit, as long as the
> patch honors the 80-column limit).
>
> > +                 continue;
>
> >                       if (i != j)
> >                               buffer->buf[j] = buffer->buf[i];
> >                       j++;
> > -             }
> > +
>
> Do we need a blank line here?  I dunno.
>
> >       }
> >       strbuf_setlen(buffer, j);
> >  }
> > @@ -1023,8 +1026,10 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,
> >       }
> >       strbuf_release(&gpg_status);
> >
> > -     /* Strip CR from the line endings, in case we are on Windows. */
> > -     remove_cr_after(signature, bottom);
> > +     /* Trim carriage returns (CR) only when they appear before line feeds (LF),.
> > +     *  mainly for handling Windows-style line endings
> > +     */
>
>         /* Convert CRLF to LF, in case we are on Windows */
>
> > +     trim_cr_before_lf(signature, bottom);
> >
> >       return 0;
> >  }
> > @@ -1110,8 +1115,10 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
> >                       ssh_signature_filename.buf);
> >               goto out;
> >       }
> > -     /* Strip CR from the line endings, in case we are on Windows. */
> > -     remove_cr_after(signature, bottom);
> > +     /* Trim carriage returns (CR) only when they appear before line feeds (LF),
> > +     *  mainly for handling Windows-style line endings.
> > +     */
> > +     trim_cr_before_lf(signature, bottom);
>
> Ditto.
>
> >
> >  out:
> >       if (key_file)