Re: nl: -d option with a single character

[Pádraig Brady]

On 14/12/2020 22:27, Bernhard Voelker wrote:
> Hi Padraig,
>
> On 12/14/20 5:53 PM, Pádraig Brady wrote:
> > I've attached two patches to address this.
>
> 'git am' complains:
>    Patch format detection failed.
> At least the usual patch end marker "---\n$GITVERSION" seems to be
> missing, but adding one doesn't help either; 'patch -p1' works though.

Sorry I attached output from `git show` rather than `git format-patch`.

> There's one more place to clarify the GNU extension:
>
>    $ nl --help
>    ...
>    -d, --section-delimiter=CC      use CC for logical page delimiters
>    ...
>    CC are two delimiter characters used to construct logical page delimiters;
>    a missing second character implies ':'.

will do

> Finally: what about 0-length section-delimiters?
>    $ nl -d ''
> Should we document this case as well?

Good point. We should document how this is handled.

> The patch also fixes this case - and now uses the whole 
> DEFAULT_SECTION_DELIMITERS:
>
>    $ printf '%s\n' a '\:' c | /usr/bin/nl -d ''
>         1       a
>         2       \:
>         3       c
>    $ printf '%s\n' a '\:' c | src/nl -d ''
>         1       a
>
>           c
>
> Therefore, `nl -d ''` is identical with `nl -d '\:'` now.  This behavior
> looks more consistent than before to me.

TBH in retrospect I agree with Kobayashi here,
that `nl -d ''` should disable section matching.
It does this in an awkward way now though,
which might access uninitialized memory.
I've cleaned this up in the attached.

You can apply the 3 diffs by using git am on the attached.

thanks for the review,
Pádraig

nl-delim-3.patch
Description: Text Data