Page MenuHomeFreeBSD
Differential D55617

yes: Completely overengineer
Needs ReviewPublic
Actions

Authored by des on Mon, Mar 2, 3:55 PM.
Tags
None
Referenced Files
F147190884: D55617.id173168.diff
Sun, Mar 8, 11:53 PM
F147177003: D55617.id173062.diff
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
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 71155
Build 68038: 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
105

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
105

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)

Revision Contents
Changeset List

PathSize
usr.bin/
yes/
6 lines
86 lines

Diff 173062

View Options

usr.bin/yes/yes.1

Loading...
View Options

usr.bin/yes/yes.c

Loading...