Re: [PATCH] status: hint the user about -uno if read_directory takes too long
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 13, 2013, 16:16 UTC
- Message-ID
- <7vehfj46mu.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1363179556-4144-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 12 quoted lines
> diff --git a/Documentation/config.txt b/Documentation/config.txt > index bbba728..e91d06f 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -178,6 +178,10 @@ advice.*:: > the template shown when writing commit messages in > linkgit:git-commit[1], and in the help message shown > by linkgit:git-checkout[1] when switching branch. > + statusUno:: > + If collecting untracked files in linkgit:git-status[1] > + takes more than 2 seconds, hint the user that the option > + `-uno` could be used to stop collecting untracked files.
It looks to me that the way this paragraph conveys information is vastly different from all the others in the section. The section begins with "by setting their corresponding variables to false various advice messages can be squelched; here are the list of variables and which advice message each of them controls", so the description should be in "variable:: which advice message" form.
The noise this introduces to the test suite is a bit irritating and makes us think twice if this really a good change.
Show 20 quoted lines
> diff --git a/wt-status.c b/wt-status.c
> index ef405d0..6fde08b 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -540,7 +540,16 @@ void wt_status_collect(struct wt_status *s)
> wt_status_collect_changes_initial(s);
> else
> wt_status_collect_changes_index(s);
> - wt_status_collect_untracked(s);
> + if (s->show_untracked_files && advice_status_uno) {
> + struct timeval tv1, tv2;
> + gettimeofday(&tv1, NULL);
> + wt_status_collect_untracked(s);
> + gettimeofday(&tv2, NULL);
> + s->untracked_in_ms =
> + (uint64_t)tv2.tv_sec * 1000 + tv2.tv_usec / 1000 -
> + ((uint64_t)tv1.tv_sec * 1000 + tv1.tv_usec / 1000);
> + } else
> + wt_status_collect_untracked(s);
> }This is not wrong per-se but it took me two reads to spot that this is not "if advise is active, do the timer but do not collect; otherwise do just collect as before". I wonder if we can structure the code a bit better to make the timing bit less loud.
Show 14 quoted lines
> static void wt_status_print_unmerged(struct wt_status *s)
> @@ -1097,6 +1106,15 @@ void wt_status_print(struct wt_status *s)
> wt_status_print_other(s, &s->untracked, _("Untracked files"), "add");
> if (s->show_ignored_files)
> wt_status_print_other(s, &s->ignored, _("Ignored files"), "add -f");
> + if (advice_status_uno && s->untracked_in_ms > 2000) {
> + status_printf_ln(s, GIT_COLOR_NORMAL,
> + _("It took %.2f seconds to collect untracked files."),
> + (float)s->untracked_in_ms / 1000);
> + status_printf_ln(s, GIT_COLOR_NORMAL,
> + _("If it happens often, you may want to use option -uno"));
> + status_printf_ln(s, GIT_COLOR_NORMAL,
> + _("to speed up by stopping displaying untracked files"));
> + }"to speed up by stopping displaying untracked files" does not look like giving a balanced suggestion. It is increasing the risk of forgetting about newly created files the user may want to add, but the risk is not properly warned.
I tend to agree that the new advice would help users if phrased in a right way. Do we want them in COLOR_NORMAL, or do we want to make them stand out a bit more (do we have COLOR_BLINK ;-)?