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

[Kent Watsen <[e-mail]>]

Hi Ben,

Erik and I addressed the regression issue raised below, and made a few other minor improvements to the non-normative `rfcfold` script.

Here is a direct link to the diff: https://tools.ietf.org/rfcdiff?url2=draft-ietf-netmod-artwork-folding-12.txt <https://tools.ietf.org/rfcdiff?url2=draft-ietf-netmod-artwork-folding-12.txt>

Please let us know if there are anymore improvements to be made.

Thanks!
Kent (and Erik)



> On Jan 11, 2020, at 9:24 AM, Erik Auerswald <[e-mail]> wrote:
> 
> 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
> 
> _______________________________________________
> netmod mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/netmod