Re: [PATCH 1/2] format-patch: have progress option while generating patches

Jeff King [off-list ref] writes:

As I said above, I think I'd prefer it to require "--progress", as
format-patch is quite often used as plumbing.

Yes, that sounds sensible.

Initially, my reaction was "Why do we even need --progress for
format-patch, when it gives one-line per patch output to show the
progress anyway?", but if that output is redirected to a file, of
course you'd need --progress independently.

Should this use start_progress_delay()? In most cases the command will
complete very quickly, and the progress report is just noise. For many
commands (e.g., checkout) we wait 1-2 seconds before bothering to show
progress output.

It is better to use the "delay" version for progress meters for
commands that may or may not last very long, and this may be a good
candidate to heed that principle.

The subcommands that use start_progress() tend to be older and more
batch oriented operations, e.g. fsck, pack-objects, etc., that are
expected to last longer.  It may be a good idea to convert them to
the "delay" variant, but obviously that is outside the scope of this
patch.

I would have expected this to say "Generating patches"; most of our
other progress messages are pluralized. You could use Q_() to handle the
mono/plural case, but I think it's fine to just always say "patches"
(that's what other messages do).

One final thought is whether callers would want to customize this
message, since it will often be used as plumbing. E.g., would rebase
want to say something besides "Generating patches". I'm not sure.
Anyway, if you're interested in that direction, there's prior art in
the way rev-list handles "--progress" (and its sole caller,
check_connected()).

These are all good suggestions.