On Sun, Nov 10, 2019 at 12:41:25PM -0800, Robin H. Johnson wrote: > Support the progress output options from pack-objects in git-bundle's > create subcommand. Most notably, this provides --quiet as requested on > the git mailing list per [1] > > Reference: https://www.mail-archive.com/git@xxxxxxxxxxxxxxx/msg182844.html <robbat2-20190806T191156-796782357Z@xxxxxxxxxxxxxxxxxx> I'm glad you included the message-id here, since "182844" is useless if mail-archive.com ever goes away. We usually just cite public-inbox for that reason, since its URLs just use the message-id anyway: https://public-inbox.org/git/robbat2-20190806T191156-796782357Z@xxxxxxxxxxxxxxxxxx > +--progress:: > + Progress status is reported on the standard error stream > + by default when it is attached to a terminal, unless -q > + is specified. This flag forces progress status even if > + the standard error stream is not directed to a terminal. > + > +--all-progress:: > + When --stdout is specified then progress report is > + displayed during the object count and compression phases > + but inhibited during the write-out phase. The reason is > + that in some cases the output stream is directly linked > + to another command which may wish to display progress > + status of its own as it processes incoming pack data. > + This flag is like --progress except that it forces progress > + report for the write-out phase as well even if --stdout is > + used. > + > +--all-progress-implied:: > + This is used to imply --all-progress whenever progress display > + is activated. Unlike --all-progress this flag doesn't actually > + force any progress display by itself. > + > +-q:: > +--quiet:: > + This flag makes the command not to report its progress > + on the standard error stream. Do we need all four of these? Just saying "--no-progress" would do what you want right now. I could understand the desire for a general "--quiet" flag that implies "--no-progress", and shuts off any other non-progress chatter as well. There isn't any now, but it could be a future proofing thing (plus having a "-q" option is standard). But I think we should document it that way from the outset (though I notice you probably just lifted this from pack-objects, IMHO it should be more clear, too). The "all-progress" thing doesn't seem useful at this level. pack-objects needs it so that it can do the right thing when being driven by upload-pack versus send-pack. But for a bundle, we're always writing to a file. We'd always want "all-progress" (and that's what the current code does). Likewise, "all-progress-implied" is about setting the "all-progress" bit but still letting pack-objects decide whether to show progress based on isatty(2). I don't think we'd need that here at all (we check isatty ourselves, and we'd always want all-progress). So could we perhaps simplify this to: 1. Set show_progress to isatty(2). 2. Make --progress a parseopt bool, setting show_progress to 1 (or if we see "--no-progress"). 3. Pass "--no-progress" or "--all-progress" to pack-objects, based on show_progress. 4. (Optional) Make "--quiet" a synonym for "--no-progress", with the documentation that it may later encompass other messages. -Peff
