Re: [PATCH v3 1/2] progress: create GIT_PROGRESS_DELAY
From: Jeff King <hidden>
Date: 2019-11-07 21:22:38
On Thu, Nov 07, 2019 at 05:46:57PM +0000, Derrick Stolee via GitGitGadget wrote:
From: Derrick Stolee <redacted> The start_delayed_progress() method is a preferred way to show optional progress to users as it ignores steps that take less than two seconds. However, this makes testing unreliable as tests expect to be very fast. In addition, users may want to decrease or increase this time interval depending on their preferences for terminal noise. Create the GIT_PROGRESS_DELAY environment variable to control the delay set during start_delayed_progress(). Set the value in some tests to guarantee their output remains consistent.
Thanks for wrapping this up. I obviously think this is a good direction to go. :) A few thoughts:
quoted hunk ↗ jump to hunk
diff --git a/Documentation/git.txt b/Documentation/git.txt index 9b82564d1a..1c420da208 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt@@ -544,6 +544,10 @@ other a pager. See also the `core.pager` option in linkgit:git-config[1]. +`GIT_PROGRESS_DELAY`:: + A number controlling how many seconds to delay before showing + optional progress indicators. Defaults to 2.
Not all progress meters use delay. I wonder if that might confuse some users, who would try: GIT_PROGRESS_DELAY=10 git repack -ad or something, but still see "Enumerating objects". I guess the key word in your documentation is "optional", but maybe it needs to be spelled out more clearly. I dunno.
quoted hunk ↗ jump to hunk
@@ -269,7 +270,12 @@ static struct progress *start_progress_delay(const char *title, uint64_t total, struct progress *start_delayed_progress(const char *title, uint64_t total) { - return start_progress_delay(title, total, 2, 0); + static int delay_in_secs = -1; + + if (delay_in_secs < 0) + delay_in_secs = git_env_ulong("GIT_PROGRESS_DELAY", 2); + + return start_progress_delay(title, total, delay_in_secs, 0); }
You asked earlier if it was worth memo-izing the git_env_ulong() call like this. I suspect it doesn't matter much either way, since progress only starts and stops a few times in a given program. But I'm certainly happy with it this way, as it matches most other environment lookups. -Peff