Page MenuHomeFreeBSD
Differential D2178

bin/sed: Implement long options for sed, add optional -Ii arguments
AbandonedPublic
Actions

Authored by eadler on Mar 31 2015, 5:09 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Sep 27, 3:11 PM
Unknown Object (File)
Tue, Sep 23, 9:41 PM
Unknown Object (File)
Thu, Sep 18, 6:15 AM
Unknown Object (File)
Wed, Sep 17, 8:42 AM
Unknown Object (File)
Mon, Sep 15, 7:14 AM
Unknown Object (File)
Sun, Sep 14, 10:37 AM
Unknown Object (File)
Sat, Sep 13, 8:41 AM
Unknown Object (File)
Sat, Sep 13, 1:16 AM
View All Files
Subscribers
antoine
emaste
imp

Details

Reviewers
jilles
antoine
pfg
adrian
bdrewery
Group Reviewers
manpages
Summary

Implement long options for the sed command

Also implement the GNU extension of permitting -I and -i to lack any extension at all

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage

Event Timeline

eadler updated this revision to Diff 4532.Mar 31 2015, 5:09 AM
eadler retitled this revision from to bin/sed: Implement long options for sed, add optional -Ii arguments.
eadler updated this object.
eadler edited the test plan for this revision. (Show Details)
eadler added reviewers: jilles, adrian.
Herald added a subscriber: imp. · View Herald TranscriptMar 31 2015, 5:09 AM
eadler updated this revision to Diff 4533.Mar 31 2015, 5:10 AM

don't include bogus stuff

eadler updated this revision to Diff 4534.Mar 31 2015, 5:12 AM

remove debug stuff

eadler added reviewers: pfg, bdrewery.Mar 31 2015, 5:14 AM
bdrewery added inline comments.Mar 31 2015, 5:23 AM
usr.bin/sed/main.c
135

The comment looks stray to me. Is there a missing --opt for it?

eadler added inline comments.Mar 31 2015, 5:25 AM
usr.bin/sed/main.c
135

there is no need for a longopt for it, since -E covers it. This comment explains why there is no longopt for it (in case someone tries to match every shortopt -> longopt).

eadler updated this revision to Diff 4535.Mar 31 2015, 5:39 AM

It rec -> It is rec

pfg edited edge metadata.Mar 31 2015, 2:46 PM

I am scared, not about the implementation, but about backwards compatibility. If one needs the GNU extension there is the gnu sed port (and more that 30 ports depend on it), but if we need the traditional behavior, sed in base is the only option.

Apparently illumos didn't care much about it but perhaps there could be an environment variable that changes behavior to non gnu.

Please have an exp ports run on this.

bdrewery edited edge metadata.Mar 31 2015, 4:47 PM
In D2178#13, @pfg wrote:

I am scared, not about the implementation, but about backwards compatibility. If one needs the GNU extension there is the gnu sed port (and more that 30 ports depend on it), but if we need the traditional behavior, sed in base is the only option.

Apparently illumos didn't care much about it but perhaps there could be an environment variable that changes behavior to non gnu.

Please have an exp ports run on this.

This is a good point. Some configure script may try 'sed --file' (or any other flag) and then assume it is GNU Sed.

Eitan can you open a PR and assign to portmgr and set the exp-run flag on it?

eadler added a comment.Mar 31 2015, 5:22 PM

Done: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=199070

emaste added a subscriber: emaste.Apr 1 2015, 2:33 PM
antoine requested changes to this revision.Apr 13 2015, 5:59 AM
antoine added a reviewer: antoine.
antoine added a subscriber: antoine.

sed looks broken, more than half of the ports tree didn't build.

A few errors:

http://pb2.nyi.freebsd.org/data/headamd64D2178-default/2015-04-12_23h59m49s/logs/errors/libXau-1.0.8_2.log
http://pb2.nyi.freebsd.org/data/headamd64D2178-default/2015-04-12_23h59m49s/logs/errors/libfontenc-1.1.2_2.log
http://pb2.nyi.freebsd.org/data/headamd64D2178-default/2015-04-12_23h59m49s/logs/errors/libxml2-2.9.2_2.log
http://pb2.nyi.freebsd.org/data/headamd64D2178-default/2015-04-12_23h59m49s/logs/errors/libICE-1.0.9,1.log
http://pb2.nyi.freebsd.org/data/headamd64D2178-default/2015-04-12_23h59m49s/logs/errors/libXdmcp-1.1.1_2.log
http://pb2.nyi.freebsd.org/data/headamd64D2178-default/2015-04-12_23h59m49s/logs/errors/qt4-qmake-4.8.6.log
http://pb2.nyi.freebsd.org/data/headamd64D2178-default/2015-04-12_23h59m49s/logs/errors/p5-Net-SSLeay-1.68.log
http://pb2.nyi.freebsd.org/data/headamd64D2178-default/2015-04-12_23h59m49s/logs/errors/libdaemon-0.14_1.log
http://pb2.nyi.freebsd.org/data/headamd64D2178-default/2015-04-12_23h59m49s/logs/errors/gmp-5.1.3_2.log
http://pb2.nyi.freebsd.org/data/headamd64D2178-default/2015-04-12_23h59m49s/logs/errors/p5-ExtUtils-Helpers-0.022_1.log

This revision now requires changes to proceed.Apr 13 2015, 5:59 AM
eadler updated this revision to Diff 4859.Apr 16 2015, 9:40 PM
eadler edited edge metadata.

remove optional -i param

eadler updated this revision to Diff 4860.Apr 16 2015, 9:41 PM
eadler edited edge metadata.

actually do required argument

eadler updated this revision to Diff 4861.Apr 16 2015, 9:42 PM

fix usage

pfg accepted this revision.Apr 16 2015, 11:16 PM
pfg edited edge metadata.

I haven't tested it but it looks good: I do prefer splitting the patch (and I was never a fan of the optional i).

BTW, we have advanced greatly in general compatibility. it would be good to see if we still need GNU sed on the 32 ports that require it currently.

jilles requested changes to this revision.Apr 17 2015, 4:19 PM
jilles edited edge metadata.

The man page and the usage message still contain optional arguments for -i and -I.

Making the argument to --in-place and --in-place-separate optional (possibly by making them independent from -i/-I from getopt_long's point of view) does not break compatibility and may be a good idea.

This revision now requires changes to proceed.Apr 17 2015, 4:19 PM
eadler added a comment.Apr 17 2015, 4:40 PM
In D2178#30, @jilles wrote:

The man page and the usage message still contain optional arguments for -i and -I.

bah! I removed most of them, but forgot synopsis and usage

Making the argument to --in-place and --in-place-separate optional (possibly by making them independent from -i/-I from getopt_long's point of view) does not break compatibility and may be a good idea

I thought about this. I think we already break compat with GNU getopt by not having a separate --separate flag.

I may try and make the long options line up a bit more with GNU

eadler updated this revision to Diff 5008.Apr 26 2015, 2:51 AM
eadler edited edge metadata.

version I tested

Herald added a reviewer: manpages. · View Herald TranscriptApr 26 2015, 2:51 AM
eadler added a comment.Apr 26 2015, 2:52 AM

I built gmp and libdaemon: both of which build for me

jilles added inline comments.Apr 27 2015, 9:48 PM
usr.bin/sed/main.c
152

This change not only enables long options, but also options after non-option arguments. In some utilities such as units(1) we have turned that off with the "+".

usr.bin/sed/sed.1
77

This looks wrong: -a causes output files not to be truncated.

GNU sed does not support this, so we can devise our own long name.

99

When the option-argument is optional, the equals sign is required so that getopt_long() knows the option-argument is present.

Something like Fl -in-place-separate Ns Bo Ns = Ns Ar extension Bc seems to work.

The same thing applies to -i/--in-place below.

eadler abandoned this revision.Apr 13 2016, 3:00 PM

Revision Contents
Changeset List

PathSize
usr.bin/
sed/
36 lines
22 lines

Diff 5008

View Options

usr.bin/sed/main.c

Loading...
View Options

usr.bin/sed/sed.1

Loading...