Jeff King <peff@xxxxxxxx> 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.