Page MenuHomeFreeBSD
Differential D55617

yes: Completely overengineer
ClosedPublic
Actions

Authored by des on Mon, Mar 2, 3:55 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 8, 11:53 PM
Unknown Object (File)
Sun, Mar 8, 9:10 PM
Unknown Object (File)
Sat, Mar 7, 2:30 PM
Unknown Object (File)
Sat, Mar 7, 8:38 AM
Unknown Object (File)
Fri, Mar 6, 9:40 PM
Unknown Object (File)
Fri, Mar 6, 11:47 AM
Unknown Object (File)
Fri, Mar 6, 11:27 AM
Unknown Object (File)
Fri, Mar 6, 8:01 AM
View All 29 Files
Subscribers
gahr
imp
kevans
ziaee

Details

Reviewers
allanjude
kevans
gahr
Group Reviewers
Klara
Commits
rGcf74b63d61b4: yes: Completely overengineer
Summary

If we're going to overengineer this, we may as well go all the way.

  • 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.

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

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 71157
Build 68040: 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
106

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
106

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.

des updated this revision to Diff 173168.Thu, Mar 5, 8:25 AM

drop getopt loop

This revision now requires review to proceed.Thu, Mar 5, 8:25 AM
Harbormaster completed remote builds in B71190: Diff 173168.Thu, Mar 5, 8:25 AM
des added a reviewer: gahr.Thu, Mar 5, 8:25 AM

I moved the getopt() loop to D55664 so this can be merged without breaking compatibility.

des added a child revision: D55664: yes: Add getopt loop.Thu, Mar 5, 8:26 AM
des edited the summary of this revision. (Show Details)
This revision was not accepted when it landed; it landed in state Needs Review.Tue, Mar 10, 10:19 AM
Closed by commit rGcf74b63d61b4: yes: Completely overengineer (authored by des). · Explain Why
This revision was automatically updated to reflect the committed changes.
des added a commit: rGcf74b63d61b4: yes: Completely overengineer.

Revision Contents
Changeset List

PathSize
usr.bin/
yes/
6 lines
88 lines

Diff 173065

View Options

usr.bin/yes/yes.1

Loading...
View Options

usr.bin/yes/yes.c

Loading...