Re: [PATCH v2 1/6] version: refactor redact_non_printables()

[Christian Couder <christian.couder@xxxxxxxxx>]

On Mon, Jan 20, 2025 at 6:10 PM Usman Akinyemi
<usmanakinyemi202@xxxxxxxxx> wrote:
>
> On Fri, Jan 17, 2025 at 11:56 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
> >
> > Usman Akinyemi <usmanakinyemi202@xxxxxxxxx> writes:

> > > +static void redact_non_printables(struct strbuf *buf)
> > > +{
> > > +     strbuf_trim(buf);
> > > +     for (size_t i = 0; i < buf->len; i++) {
> > > +             if (buf->buf[i] <= 32 || buf->buf[i] >= 127)
> >
> > <sane-ctype.h> defines isprint() we can use here.
> I think it would be better to add this in another commit so that one commit
> does one thing. I will add it after this patch series got settled,
> what do you think ?

Alternatively it could be done in its own preparatory patch at the
beginning of this patch series.

<sane-ctype.h> has:

#define isprint(x) ((x) >= 0x20 && (x) <= 0x7e)

So if we wanted to use isprint() we would have to use something like:

    for (size_t i = 0; i < buf->len; i++) {
            if (!isprint(buf->buf[i]) || buf->buf[i] == ' ')
                    buf->buf[i] = '.';
    }

It would have been nicer if we didn't need a special case for SP. So I
would say it's likely a matter of taste if the result is nicer than
the original.

> >
> > > +                     buf->buf[i] = '.';
> > > +     }
> > > +}