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
[email protected]
https://www.ietf.org/mailman/listinfo/netmod

Reply via email to