Re: nl: -d option with a single character

[KOBAYASHI Takashi]

Thanks!

> It does this in an awkward way now though,
> which might access uninitialized memory.
> I've cleaned this up in the attached.

Looks good to me.

Takashi

2020年12月14日(月) 17:19 Pádraig Brady <P@draigbrady.com>:

> 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
>