Re: [netmod] Benjamin Kaduk's Discuss on draft-ietf-netmod-artwork-folding-11: (with DISCUSS and COMMENT)

[Erik Auerswald <[e-mail]>]

Hello Benjamin,

On 10.01.20 00:28, Benjamin Kaduk via Datatracker wrote:
> Benjamin Kaduk has entered the following ballot position for
> draft-ietf-netmod-artwork-folding-11: Discuss
> 
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
> 
> 
> Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> for more information about IESG DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-netmod-artwork-folding/
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> Thank you for the updates in the -10 and -11; the content looks a lot better and
> I am not uncomfortable about publishing as Informational (vs. BCP)!

Thanks for the review. :-)

> That said, I think the edits to the script have introduced a regression:
> 
>       # ensure input file doesn't contain the fold-sequence already
>       if [[ -n "$("$SED" -n '/\\$/{N;s/\\\n[ ]*\\/&/p}' "$infile")" ]]
> 
> Unfortunately, I'm not sure this gets all cases, since the 'N' command
> reads a line and prevents it from being considered as the first half of the
> wrapped sequence:
> 
> kaduk$:~/git/openssl$ cat /tmp/a
> this is a line\
> another line\
> \that wraps
> kaduk$:~/git/openssl$ cat /tmp/b
> this is a line
> another line\
> \that wraps
> kaduk$:~/git/openssl$ sed -n '/\\$/{N;s/\\\n[ ]*\\/&/p}' < /tmp/a
> kaduk$:~/git/openssl$ sed -n '/\\$/{N;s/\\\n[ ]*\\/&/p}' < /tmp/b
> another line\
> \that wraps

Thanks for the bug report complete with test cases and analysis. :-)
The fix should be adding ";D" to the sed script:

     sed -n '/\\$/{N;s/\\\n[ ]*\\/&/p;D}'

I'll look into adding this (with tests) soon.

> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> A few other comments from reviewing the version of the script in the -11:
> 
> When processing input, it's perhaps more robust to check $# before assigning $2
> to a named parameter.

That sounds reasonable.

>       printf "Exit status code: 1 on error, 0 on success, 255 on no-op."
> 
> Interesting to have no newline here but two on the next line's printf, but I
> guess it might be at the column limit already.

Exactly.

> (The quotes on 'Error'/'Warning'/'Debug' in err()/warn()/dbg() are noops.)

They improve syntax-highlighting results in vim. ;-)

>     # warn if a non-GNU sed utility is used
>     "$SED" --version < /dev/null 2> /dev/null \
>     | grep GNU > /dev/null 2>&1 || \
> 
> `grep -q` should be usable instead of `grep >/dev/null`

I intend to change this to use `grep -q`.

Thanks,
Erik