[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: 3 minor sed adjustment
From: |
Jim Meyering |
Subject: |
Re: 3 minor sed adjustment |
Date: |
Mon, 1 Aug 2016 10:41:19 -0700 |
On Sat, Jul 30, 2016 at 11:46 PM, Assaf Gordon <address@hidden> wrote:
> Hello,
>
> Attached are 3 patches for minor sed adjustments - I think they make sed
> behave more consistently:
>
> Assaf Gordon (3):
> sed: allow multiple (non-conflicting) -E/-r parameters
> sed: adjust line-terminator of F/l/= commands when -z is used
> sed: reject 's///e' in --posix mode
>
> sed/compile.c | 2 ++
> sed/execute.c | 16 ++++++++++------
> sed/sed.c | 6 ++++--
> testsuite/nulldata.sh | 22 +++++++++++++++-------
> testsuite/posix-mode-s.sh | 9 +++++++--
> 5 files changed, 38 insertions(+), 17 deletions(-)
Thank you.
I like all three.
The first one is fine, but maybe we should talk about whether to
eventually add PERL support.
I am leaning against that, partly due to my experience with -P support
in grep, and partly just because one can use perl -pe or perl -pi -e
itself more portably than GNU sed with an eventual perl mode.
In the second patch, this change
if (width+olen >= line_len && line_len > 0) {
- ck_fwrite("\\\n", 1, 2, fp);
+ ck_fwrite("\\", 1, 1, fp);
+ ck_fwrite(&buffer_delimiter, 1, 1, fp);
appears to change from emitting backslash-NL-continued lines to
backslash-NUL with -z. When using -z, do you still want to emit that
backslash?
Note that this is in code to honor sed's --line-length=N (-l) option,
which one can argue is not relevant with -z.
In the third one, there is an "export LC_ALL=C" line.
That should not be needed, given the LC_ALL=C in testsuite/local.mk.
Assuming you adjust that, would you also remove the preceding
(preexisting) unset POSIXLY_CORRECT? That too is unnecessary, since
the change that added the envvar-check code that is run before every
test. It ensures that envvar is not set.
- Re: 3 minor sed adjustment,
Jim Meyering <=