On Fri, Jul 10, 2020 at 02:14:51AM +0000, brian m. carlson wrote: > > On 2020-07-10 at 01:42:41, Emily Shaffer wrote: > > Before now, the progress API is used by conditionally calling > > start_progress() or a similar call, and then unconditionally calling > > display_progress() and stop_progress(), both of which are tolerant of > > NULL or uninitialized inputs. However, in > > 98a136474082cdc7228d7e0e45672c5274fab701 (trace: log progress time and > > throughput), the progress library learned to log traces during expensive > > operations. In cases where progress should not be displayed to the user > > - such as when Git is called by a script - no traces will be logged, > > because the progress object is never created. > > > > Instead, to allow us to collect traces from scripted Git commands, teach > > a progress->verbose flag, which is specified via a new argument to > > start_progress() and friends. display_progress() also learns to filter > > for that flag. With these changes, start_progress() can be called > > unconditionally but with a conditional as an argument to determine > > whether to report progress to the user. > > So to make sure I understand this right, we'll collect traces regardless > if it's enabled, but we'll still honor the --quiet flag if the user > doesn't want to see them? Yes, precisely. display_progress() used to check whether to display or not based on whether the 'struct progress' was valid; now it also checks whether 'progress->verbose' is set. display() (where the real logic lives for printing to the user) still never gets called with this change. > > If so, I'm definitely in favor of this > change. I was worried when I read the cover letter that we'd display > them to the user regardless, but from reading the patch and the commit > message, it seems I misunderstood. > > I think the making the verbose flag a parameter simplifies the code > nicely and puts the rendering decision in the right place. Thanks! - Emily
