Page MenuHomeFreeBSD
Differential D55617

yes: Completely overengineer
AcceptedPublic
Actions

Authored by des on Mon, Mar 2, 3:55 PM.
Tags
None
Referenced Files
F146698573: D55617.id173065.diff
Wed, Mar 4, 7:49 PM
F146692617: D55617.id173017.diff
Wed, Mar 4, 6:40 PM
F146684907: D55617.id.diff
Wed, Mar 4, 5:17 PM
F146649221: D55617.id173065.diff
Wed, Mar 4, 10:16 AM
F146645233: D55617.id173062.diff
Wed, Mar 4, 9:25 AM
F146645200: D55617.id173062.diff
Wed, Mar 4, 9:25 AM
F146613547: D55617.id173016.diff
Wed, Mar 4, 2:53 AM
F146613532: D55617.id173056.diff
Wed, Mar 4, 2:53 AM
View All 19 Files
Subscribers
gahr
imp
kevans
ziaee

Details

Reviewers
allanjude
kevans
Group Reviewers
Klara
Summary

If we're going to overengineer this, we may as well go all the way.
Well, most of the way. I didn't feel like adding long options.

  • Add a getopt() loop and a usage message.
  • If multiple arguments are given, concatenate them into a space- separated list like GNU coreutils does.
  • When duplicating the expletive, do so exponentially.
  • Most importantly, don't modify the memory that argv points to.

This will now behave mostly like GNU coreutils, except for the absence
of the --help and --version options.

MFC after: 1 week
Sponsored by: Klara, Inc.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 71134
Build 68017: arc lint + arc unit

Event Timeline

des created this revision.Mon, Mar 2, 3:55 PM
Herald added subscribers: ziaee, imp. · View Herald TranscriptMon, Mar 2, 3:55 PM
des requested review of this revision.Mon, Mar 2, 3:55 PM
Harbormaster completed remote builds in B71134: Diff 173016.Mon, Mar 2, 3:55 PM
des updated this revision to Diff 173017.Mon, Mar 2, 3:56 PM

drop ktrace

Harbormaster completed remote builds in B71135: Diff 173017.Mon, Mar 2, 3:57 PM
des edited the summary of this revision. (Show Details)Mon, Mar 2, 3:57 PM
des added a reviewer: gahr.
des updated this revision to Diff 173018.Mon, Mar 2, 3:59 PM

drop debugging printf

Harbormaster completed remote builds in B71136: Diff 173018.Mon, Mar 2, 3:59 PM
gahr added a comment.Mon, Mar 2, 10:01 PM

This changes yes -foo from emitting -foo to an error. Do we really want that for the sake of compatibility with GNU's version?

gahr requested changes to this revision.Mon, Mar 2, 10:06 PM
This revision now requires changes to proceed.Mon, Mar 2, 10:06 PM
gahr added inline comments.Mon, Mar 2, 10:07 PM
usr.bin/yes/yes.c
95

Leftover?

des added a comment.Tue, Mar 3, 11:58 AM
In D55617#1272796, @gahr wrote:

This changes yes -foo from emitting -foo to an error. Do we really want that for the sake of compatibility with GNU's version?

That's not just “compatibility with GNU”, it's consistency with our other command-line utilities (except df and echo, which are special for historical reasons).

usr.bin/yes/yes.c
95

Yes. I thought I'd removed it...

des updated this revision to Diff 173056.Tue, Mar 3, 12:00 PM

really remove debugging printf

Harbormaster completed remote builds in B71151: Diff 173056.Tue, Mar 3, 12:00 PM
des marked an inline comment as done.Tue, Mar 3, 12:00 PM
gahr added a comment.Tue, Mar 3, 1:14 PM
In D55617#1272905, @des wrote:
In D55617#1272796, @gahr wrote:

This changes yes -foo from emitting -foo to an error. Do we really want that for the sake of compatibility with GNU's version?

That's not just “compatibility with GNU”, it's consistency with our other command-line utilities (except df and echo, which are special for historical reasons).

Well, it looks like yes is special for historical reasons too.. I don't see a value in this change, and I see a diminished functionality. But I won't stand in the way if you find consensus.

des updated this revision to Diff 173062.Tue, Mar 3, 5:03 PM

fix perf regression

Harbormaster completed remote builds in B71155: Diff 173062.Tue, Mar 3, 5:03 PM
des removed a reviewer: gahr.Tue, Mar 3, 5:04 PM
des added a subscriber: gahr.
In D55617#1272925, @gahr wrote:

Well, it looks like yes is special for historical reasons too.. I don't see a value in this change, and I see a diminished functionality. But I won't stand in the way if you find consensus.

df and echo are documented (and required) to deviate from the standard command-line conventions; yes is not.

des updated this revision to Diff 173065.Tue, Mar 3, 5:24 PM

rename MINBUF to OPTBUF and explain

Harbormaster completed remote builds in B71157: Diff 173065.Tue, Mar 3, 5:24 PM
allanjude accepted this revision.Tue, Mar 3, 5:26 PM
This revision is now accepted and ready to land.Tue, Mar 3, 5:26 PM
kevans accepted this revision.Wed, Mar 4, 2:28 PM
kevans added a subscriber: kevans.

Happy with this as long as we tag the getopt-related incompatibility for relnotes. I suspect it's not a problem in practice, but we should still call it out.

Revision Contents
Changeset List

PathSize
usr.bin/
yes/
6 lines
73 lines

Diff 173016

View Options

usr.bin/yes/ktrace.out

Loading...
View Options

usr.bin/yes/yes.1

Loading...
View Options

usr.bin/yes/yes.c

Loading...