Re: [PATCH v3 1/5] version: refactor strbuf_sanitize()

[Christian Couder <christian.couder@gmail.com>]

On Sat, Dec 7, 2024 at 7:21 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Christian Couder <christian.couder@gmail.com> writes:
>
> > +/*
> > + * Trim and replace each character with ascii code below 32 or above
> > + * 127 (included) using a dot '.' character. Useful for sending
> > + * capabilities.
> > + */
> > +void strbuf_sanitize(struct strbuf *sb);
>
> I am not getting "Useful for sending capabilities" here, and feel
> that it is somewhat an unsubstantiated claim.  If some information
> is going to be transferred (which the phrase "sending capabilities"
> hints), I'd expect that we try as hard as possible not to lose
> information, but redact-non-ASCII is the total opposite of "not
> losing information".

Ok, "Useful for sending capabilities" will be removed.

> By the way, as we are trimming, I am very very much tempted to
> squish a run of non-ASCII bytes into one dot, perhaps like
>
>         void redact_non_printables(struct strbuf *sb)
>         {
>                 size_t dst = 0;
>                 int skipped = 0;
>
>                 strbuf_trim(sb);
>                 for (size_t src = 0; src < sb->len; src++) {
>                         int ch = sb->buf[src];
>                         if (ch <= 32 && 127 <= ch) {
>                                 if (skipped)
>                                         continue;
>                                 ch = '.';
>                         }
>                         sb->buf[dst++] = ch;
>                         skipped = (ch == '.');
>                 }
>         }
>
> or even without strbuf_trim(), which would turn any leading or
> trailing run of whitespaces into '.'.
>
> But that is an improvement that can be easily done on top after the
> dust settles and better left as #leftoverbits material.

Usman's patch series about introducing a "os-version" capability needs
such a feature too, and Usman already reworked this code according to
your comments here. It looks like you found it good too. So I will
just reuse his patches related to this in the version 4 of this patch
series.