Details

Reviewers

allanjude
kevans
gahr

Group Reviewers

Klara

Commits

rGf2e600ff156d: yes: Completely overengineer
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 71151
Build 68034: 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

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.

drop debugging printf

Harbormaster completed remote builds in B71136: Diff 173018.Mon, Mar 2, 3:59 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?

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

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

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.

fix perf regression

Harbormaster completed remote builds in B71155: Diff 173062.Tue, Mar 3, 5:03 PM

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.

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

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.

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

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.

des added a commit: rGf2e600ff156d: yes: Completely overengineer.Mon, Mar 23, 3:08 PM