git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Rudy Rigot via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Rudy Rigot <rudy.rigot@gmail.com>, Rudy Rigot <rudy.rigot@gmail.com>
Subject: [PATCH] fsmonitor: long status advice adapted to the fsmonitor use case
Date: Sat, 15 Oct 2022 13:04:10 +0000	[thread overview]
Message-ID: <pull.1384.git.1665839050813.gitgitgadget@gmail.com> (raw)

From: Rudy Rigot <rudy.rigot@gmail.com>

Currently, if git-status takes more than 2 seconds for enumerating
untracked files, a piece of advice is given to the user to consider
ignoring untracked files. This is somewhat at odds with the UX
upsides from having fsmonitor enabled, since fsmonitor will be
here to take care of mitigating the performance downsides from
those untracked files.

I considered just suppressing that piece of advice entirely for
repositories with fsmonitor disabled, but I decided to replace
it with another piece of advice instead, letting the user know
that this run may have been slow, but the next ones should be faster.
Of course, please let me know if the phrasing can be improved. To
keep consistent with other pieces of advice, this new one can be
hidden with a new advice.statusFsmonitor config.

If the repository does not have fsmonitor enabled, or if the new
piece of advice is hidden by config, the behavior falls back to
today's behavior: show the message advising to ignore untracked
files, as long as it wasn't disabled with the existing advice.statusUoption
config.

Test-wise, I tried to figure out ways to mock the behavior of a
slow git-status, but I couldn't figure it out, so I could use some
advice. I tracked down Commit 6a38ef2ced (status: advise to consider
use of -u when read_directory takes too long, 2013-03-13), and it
also didn't have tests, so I'm questioning whether it can in fact
be reasonably done. Thanks in advance for any guidance.

Signed-off-by: Rudy Rigot <rudy.rigot@gmail.com>
---
    fsmonitor: long status advice adapted to the fsmonitor use case
    
    Currently, if git-status takes more than 2 seconds for enumerating
    untracked files, a piece of advice is given to the user to consider
    ignoring untracked files. This is somewhat at odds with the UX upsides
    from having fsmonitor enabled, since fsmonitor will be here to take care
    of mitigating the performance downsides from those untracked files.
    
    I considered just suppressing that piece of advice entirely for
    repositories with fsmonitor disabled, but I decided to replace it with
    another piece of advice instead, letting the user know that this run may
    have been slow, but the next ones should be faster. Of course, please
    let me know if the phrasing can be improved. To keep consistent with
    other pieces of advice, this new one can be hidden with a new
    advice.statusFsmonitor config.
    
    If the repository does not have fsmonitor enabled, or if the new piece
    of advice is hidden by config, the behavior falls back to today's
    behavior: show the message advising to ignore untracked files, as long
    as it wasn't disabled with the existing advice.statusUoption config.
    
    Test-wise, I tried to figure out ways to mock the behavior of a slow
    git-status, but I couldn't figure it out, so I could use some advice. I
    tracked down Commit 6a38ef2 (status: advise to consider use of -u when
    read_directory takes too long, 2013-03-13), and it also didn't have
    tests, so I'm questioning whether it can in fact be reasonably done.
    Thanks in advance for any guidance.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1384%2Frudyrigot%2Fadvice_statusFsmonitor-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1384/rudyrigot/advice_statusFsmonitor-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1384

 Documentation/config/advice.txt |  3 +++
 advice.c                        |  1 +
 advice.h                        |  1 +
 wt-status.c                     | 23 ++++++++++++++++-------
 4 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
index a00d0100a82..e8ebcf1b023 100644
--- a/Documentation/config/advice.txt
+++ b/Documentation/config/advice.txt
@@ -57,6 +57,9 @@ advice.*::
 		and that calculation takes longer than expected. Will not
 		appear if `status.aheadBehind` is false or the option
 		`--no-ahead-behind` is given.
+	statusFsmonitor::
+		Shown when the linkgit:git-status[1] command takes more than
+		2 seconds to enumerate untracked files, and fsmonitor is enabled.
 	statusHints::
 		Show directions on how to proceed from the current
 		state in the output of linkgit:git-status[1], in
diff --git a/advice.c b/advice.c
index fd189689437..448b11ef273 100644
--- a/advice.c
+++ b/advice.c
@@ -69,6 +69,7 @@ static struct {
 	[ADVICE_SET_UPSTREAM_FAILURE]			= { "setUpstreamFailure", 1 },
 	[ADVICE_SKIPPED_CHERRY_PICKS]			= { "skippedCherryPicks", 1 },
 	[ADVICE_STATUS_AHEAD_BEHIND_WARNING]		= { "statusAheadBehindWarning", 1 },
+	[ADVICE_STATUS_FSMONITOR]			= { "statusFsmonitor", 1 },
 	[ADVICE_STATUS_HINTS]				= { "statusHints", 1 },
 	[ADVICE_STATUS_U_OPTION]			= { "statusUoption", 1 },
 	[ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE] = { "submoduleAlternateErrorStrategyDie", 1 },
diff --git a/advice.h b/advice.h
index 07e0f76833e..a458619a160 100644
--- a/advice.h
+++ b/advice.h
@@ -43,6 +43,7 @@ struct string_list;
 	ADVICE_SEQUENCER_IN_USE,
 	ADVICE_SET_UPSTREAM_FAILURE,
 	ADVICE_STATUS_AHEAD_BEHIND_WARNING,
+	ADVICE_STATUS_FSMONITOR,
 	ADVICE_STATUS_HINTS,
 	ADVICE_STATUS_U_OPTION,
 	ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE,
diff --git a/wt-status.c b/wt-status.c
index 5813174896c..d6c7f0fa21a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -18,6 +18,7 @@
 #include "worktree.h"
 #include "lockfile.h"
 #include "sequencer.h"
+#include "fsmonitor-settings.h"
 
 #define AB_DELAY_WARNING_IN_MS (2 * 1000)
 
@@ -1814,6 +1815,7 @@ static void wt_longstatus_print(struct wt_status *s)
 {
 	const char *branch_color = color(WT_STATUS_ONBRANCH, s);
 	const char *branch_status_color = color(WT_STATUS_HEADER, s);
+	enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(s->repo);
 
 	if (s->branch) {
 		const char *on_what = _("On branch ");
@@ -1870,13 +1872,20 @@ static void wt_longstatus_print(struct wt_status *s)
 		wt_longstatus_print_other(s, &s->untracked, _("Untracked files"), "add");
 		if (s->show_ignored_mode)
 			wt_longstatus_print_other(s, &s->ignored, _("Ignored files"), "add -f");
-		if (advice_enabled(ADVICE_STATUS_U_OPTION) && 2000 < s->untracked_in_ms) {
-			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
-			status_printf_ln(s, GIT_COLOR_NORMAL,
-					 _("It took %.2f seconds to enumerate untracked files. 'status -uno'\n"
-					   "may speed it up, but you have to be careful not to forget to add\n"
-					   "new files yourself (see 'git help status')."),
-					 s->untracked_in_ms / 1000.0);
+		if (2000 < s->untracked_in_ms) {
+			if (advice_enabled(ADVICE_STATUS_FSMONITOR) && fsm_mode > FSMONITOR_MODE_DISABLED) {
+				status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
+				status_printf_ln(s, GIT_COLOR_NORMAL,
+						_("It took a while to check your git status this time, but the results\n"
+						"were cached, and your next runs should be faster."));
+			} else if (advice_enabled(ADVICE_STATUS_U_OPTION)) {
+				status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
+				status_printf_ln(s, GIT_COLOR_NORMAL,
+						_("It took %.2f seconds to enumerate untracked files. 'status -uno'\n"
+						"may speed it up, but you have to be careful not to forget to add\n"
+						"new files yourself (see 'git help status')."),
+						s->untracked_in_ms / 1000.0);
+			}
 		}
 	} else if (s->committable)
 		status_printf_ln(s, GIT_COLOR_NORMAL, _("Untracked files not listed%s"),

base-commit: d420dda0576340909c3faff364cfbd1485f70376
-- 
gitgitgadget

             reply	other threads:[~2022-10-15 13:04 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-15 13:04 Rudy Rigot via GitGitGadget [this message]
2022-10-15 13:08 ` [PATCH] fsmonitor: long status advice adapted to the fsmonitor use case Rudy Rigot
2022-10-17 15:39 ` Jeff Hostetler
2022-10-17 16:59   ` Rudy Rigot
2022-10-20 12:56     ` Jeff Hostetler
2022-10-20 20:17       ` Rudy Rigot
2022-10-24 14:55         ` Jeff Hostetler
2022-10-29  0:06 ` [PATCH v2] status: long status advice adapted to recent capabilities Rudy Rigot via GitGitGadget
2022-11-02 19:45   ` Jeff Hostetler
2022-11-02 20:34     ` Rudy Rigot
2022-11-02 23:59     ` Taylor Blau
2022-11-03 14:28       ` Rudy Rigot
2022-11-04  8:52         ` Ævar Arnfjörð Bjarmason
2022-11-04 15:33           ` Rudy Rigot
2022-11-04 21:38         ` Taylor Blau
2022-11-02 21:27   ` [PATCH v3] " Rudy Rigot via GitGitGadget
2022-11-04 21:40     ` Taylor Blau
2022-11-07 20:02       ` Derrick Stolee
2022-11-07 23:19         ` Taylor Blau
2022-11-15 16:38       ` Jeff Hostetler
2022-11-07 20:01     ` Derrick Stolee
2022-11-07 20:20       ` Eric Sunshine
2022-11-07 20:31         ` Rudy Rigot
2022-11-10  4:46     ` [PATCH v4] " Rudy Rigot via GitGitGadget
2022-11-10  5:42       ` Eric Sunshine
2022-11-10 17:01         ` Rudy Rigot
2022-11-10 17:30           ` Eric Sunshine
2022-11-10 17:47             ` Rudy Rigot
2022-11-10 20:04       ` [PATCH v5] " Rudy Rigot via GitGitGadget
2022-11-15 16:39         ` Jeff Hostetler
2022-11-15 16:42           ` Rudy Rigot
2022-11-15 17:26         ` Eric Sunshine
2022-11-15 17:45           ` Rudy Rigot
2022-11-15 18:06             ` Eric Sunshine
2022-11-15 18:08               ` Rudy Rigot
2022-11-15 21:19         ` [PATCH v6] " Rudy Rigot via GitGitGadget
2022-11-21  5:06           ` Eric Sunshine
2022-11-21 15:54             ` Rudy Rigot
2022-11-21 16:17               ` Eric Sunshine
2022-11-22 16:52                 ` Rudy Rigot
2022-11-22 17:18                   ` Eric Sunshine
2022-11-22 17:24                     ` Eric Sunshine
2022-11-22 17:29                       ` Rudy Rigot
2022-11-22 17:40                     ` Eric Sunshine
2022-11-22 18:07                       ` Eric Sunshine
2022-11-22 19:19                         ` Rudy Rigot
2022-11-22 19:48                           ` Eric Sunshine
2022-11-22 16:59           ` [PATCH v7] status: modernize git-status "slow untracked files" advice Rudy Rigot via GitGitGadget
2022-11-22 22:07             ` [PATCH v8] " Rudy Rigot via GitGitGadget
2022-11-25  4:58               ` Junio C Hamano
2022-11-29 15:21                 ` Rudy Rigot
2022-11-30  0:51                   ` Rudy Rigot
2022-11-30  0:52               ` [PATCH v9] " Rudy Rigot via GitGitGadget
2022-12-01  6:48                 ` Junio C Hamano
2022-12-01 15:16                   ` Rudy Rigot
2022-12-01 22:45                     ` Junio C Hamano
2022-12-01 22:57                       ` Rudy Rigot
2023-05-11  5:17               ` [PATCH v8] " Eric Sunshine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1384.git.1665839050813.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=rudy.rigot@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).