Re: [PATCH] CodingGuidelines: use octal escapes, not hex

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jun 13, 2023, 19:15 UTC
Message-ID: <CAPig+cQnyL8-9EHVe5P_oRbUdsXA9WBiK_W4LHq7HOF4-ek5Pg@mail.gmail.com>
In-Reply-To: <20230613184304.36482-1-jonathantanmy@google.com>

On Tue, Jun 13, 2023 at 2:43 PM Jonathan Tan <jonathantanmy@google.com> wrote:

Show 23 quoted lines

> Eric Sunshine <sunshine@sunshineco.com> writes:
> > On Tue, Jun 13, 2023 at 1:44 PM Jonathan Tan <jonathantanmy@google.com> wrote:
> > > + - Use octal escape sequences (e.g. "\302\242"), not hexadecimal (e.g.
> > > +   "\xc2\xa2"), as the latter is not portable.
> >
> > The shell itself doesn't interpret these sequences, so this
> > description feels too generic. Perhaps it would make more sense to
> > cite specific tools for which octal sequences are needed for
> > portability reasons, such as `printf`, `sed`, `tr`, etc.
>
> Ah...good point. I checked with "echo" in "dash" and assumed that it
> was "dash" that was interpreting the escapes, but indeed it is the
> "echo" (and "printf") builtins in "dash" that are actually interpreting
> them. What do you think of the following in the commit message:
>
>   Hexadecimal escapes in shell scripts are not portable across shell builtins (in
>   particular, the "printf" of "dash" does not support them). Write in the CodingGuidelines
>   document that we should be using octal escapes instead.
>
> and in the CodingGuidelines doc:
>
> + - Use octal escape sequences (e.g. "\302\242"), not hexadecimal (e.g.
> +   "\xc2\xa2"), as the latter is not portable across some shell builtins like printf.

The portability concern is not specific to a certain shell or whether a command is builtin, so it feels misleading to single out "dash" and builtins. The same portability problems can crop up, as well, with older (non-"dash") shells, and with commands which may or may not be builtins (such as "echo" which, historically, was not always a builtin), and non-builtins commands, such as "sed" and "tr".

So, for the commit message, perhaps simply:

    Extend the shell-scripting section of CodingGuidelines to suggest
    octal escape sequences (e.g. "\302\242") over hexadecimal
    (e.g. "\xc2\xa2") since the latter can be a source of portability
    problems.

As for the change to CodingGuidelines, this would probably be sufficient:

    Use octal escape sequences (e.g. "\302\242"), not hexadecimal
    (e.g. "\xc2\xa2"), since the latter is not portable across some
    commands, such as `printf`, `sed`, `tr`, etc.