Re: [PATCH 1/2] progress: create progress struct in 'verbose' mode

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Jul 10, 2020, 02:14 UTC
Message-ID: <20200710021451.GI9782@camp.crustytoothpaste.net>
In-Reply-To: <20200710014242.1088216-2-emilyshaffer@google.com>

On 2020-07-10 at 01:42:41, Emily Shaffer wrote:

Show 16 quoted lines

> 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? 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.

-- 
brian m. carlson: Houston, Texas, US