diff mbox series

[04/10] status: skip sparse-checkout percentage with sparse-index

Message ID e86f874dd41291da66848068e7725a172dee231e.1618322497.git.gitgitgadget@gmail.com (mailing list archive)
State New
Headers show
Series Sparse-index: integrate with status and add | expand

Commit Message

Derrick Stolee April 13, 2021, 2:01 p.m. UTC
From: Derrick Stolee <dstolee@microsoft.com>

'git status' began reporting a percentage of populated paths when
sparse-checkout is enabled in 051df3cf (wt-status: show sparse
checkout status as well, 2020-07-18). This percentage is incorrect when
the index has sparse directories. It would also be expensive to
calculate as we would need to parse trees to count the total number of
possible paths.

Avoid the expensive computation by simplifying the output to only report
that a sparse checkout exists, without the percentage.

This change is the reason we use 'git status --porcelain=v2' in
t1092-sparse-checkout-compatibility.sh. We don't want to ensure that
this message is equal across both modes, but instead just the important
information about staged, modified, and untracked files are compared.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 t/t1092-sparse-checkout-compatibility.sh |  8 ++++++++
 wt-status.c                              | 14 +++++++++++---
 wt-status.h                              |  1 +
 3 files changed, 20 insertions(+), 3 deletions(-)

Comments

Elijah Newren April 20, 2021, 11:26 p.m. UTC | #1
On Tue, Apr 13, 2021 at 7:01 AM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Derrick Stolee <dstolee@microsoft.com>
>
> 'git status' began reporting a percentage of populated paths when
> sparse-checkout is enabled in 051df3cf (wt-status: show sparse
> checkout status as well, 2020-07-18). This percentage is incorrect when
> the index has sparse directories. It would also be expensive to
> calculate as we would need to parse trees to count the total number of
> possible paths.
>
> Avoid the expensive computation by simplifying the output to only report
> that a sparse checkout exists, without the percentage.

Makes sense.  The percentage wasn't critical, it was just a nice UI
bonus.  The critical part is notifying about being in a sparse
checkout.

It makes me wonder slightly if we'd want to remove the percentage for
both modes just to keep them more similar.  I'll ask some folks for
their thoughts/opinions.  Of course, that could always be tweaked
later and doesn't necessarily need to go into your series.

> This change is the reason we use 'git status --porcelain=v2' in
> t1092-sparse-checkout-compatibility.sh. We don't want to ensure that
> this message is equal across both modes, but instead just the important
> information about staged, modified, and untracked files are compared.
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  t/t1092-sparse-checkout-compatibility.sh |  8 ++++++++
>  wt-status.c                              | 14 +++++++++++---
>  wt-status.h                              |  1 +
>  3 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> index 6598c12a2069..e488ef9bd941 100755
> --- a/t/t1092-sparse-checkout-compatibility.sh
> +++ b/t/t1092-sparse-checkout-compatibility.sh
> @@ -196,6 +196,14 @@ test_expect_success 'status with options' '
>         test_all_match git status --porcelain=v2 -uno
>  '
>
> +test_expect_success 'status reports sparse-checkout' '
> +       init_repos &&
> +       git -C sparse-checkout status >full &&
> +       git -C sparse-index status >sparse &&
> +       test_i18ngrep "You are in a sparse checkout with " full &&
> +       test_i18ngrep "You are in a sparse checkout." sparse
> +'
> +
>  test_expect_success 'add, commit, checkout' '
>         init_repos &&
>
> diff --git a/wt-status.c b/wt-status.c
> index 0c8287a023e4..0425169c1895 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1490,9 +1490,12 @@ static void show_sparse_checkout_in_use(struct wt_status *s,
>         if (s->state.sparse_checkout_percentage == SPARSE_CHECKOUT_DISABLED)
>                 return;
>
> -       status_printf_ln(s, color,
> -                        _("You are in a sparse checkout with %d%% of tracked files present."),
> -                        s->state.sparse_checkout_percentage);
> +       if (s->state.sparse_checkout_percentage == SPARSE_CHECKOUT_SPARSE_INDEX)
> +               status_printf_ln(s, color, _("You are in a sparse checkout."));
> +       else
> +               status_printf_ln(s, color,
> +                               _("You are in a sparse checkout with %d%% of tracked files present."),
> +                               s->state.sparse_checkout_percentage);
>         wt_longstatus_print_trailer(s);
>  }
>
> @@ -1650,6 +1653,11 @@ static void wt_status_check_sparse_checkout(struct repository *r,
>                 return;
>         }
>
> +       if (r->index->sparse_index) {
> +               state->sparse_checkout_percentage = SPARSE_CHECKOUT_SPARSE_INDEX;
> +               return;
> +       }
> +
>         for (i = 0; i < r->index->cache_nr; i++) {
>                 struct cache_entry *ce = r->index->cache[i];
>                 if (ce_skip_worktree(ce))
> diff --git a/wt-status.h b/wt-status.h
> index 0d32799b28e1..ab9cc9d8f032 100644
> --- a/wt-status.h
> +++ b/wt-status.h
> @@ -78,6 +78,7 @@ enum wt_status_format {
>  };
>
>  #define SPARSE_CHECKOUT_DISABLED -1
> +#define SPARSE_CHECKOUT_SPARSE_INDEX -2
>
>  struct wt_status_state {
>         int merge_in_progress;
> --
> gitgitgadget

Looks good.
Derrick Stolee April 21, 2021, 1:51 p.m. UTC | #2
On 4/20/2021 7:26 PM, Elijah Newren wrote:
> On Tue, Apr 13, 2021 at 7:01 AM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>> Avoid the expensive computation by simplifying the output to only report
>> that a sparse checkout exists, without the percentage.
> 
> Makes sense.  The percentage wasn't critical, it was just a nice UI
> bonus.  The critical part is notifying about being in a sparse
> checkout.
> 
> It makes me wonder slightly if we'd want to remove the percentage for
> both modes just to keep them more similar.  I'll ask some folks for
> their thoughts/opinions.  Of course, that could always be tweaked
> later and doesn't necessarily need to go into your series.

I find the percentage helpful for users who are exploring the
sparse-checkout feature in their repositories. It's nice to know how
much time it is saving, because "percentage of files" frequently
translates to "percentage of time it takes to update the worktree".

I was sad to lose it here, but I don't see any way to keep it.

Thanks,
-Stolee
diff mbox series

Patch

diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
index 6598c12a2069..e488ef9bd941 100755
--- a/t/t1092-sparse-checkout-compatibility.sh
+++ b/t/t1092-sparse-checkout-compatibility.sh
@@ -196,6 +196,14 @@  test_expect_success 'status with options' '
 	test_all_match git status --porcelain=v2 -uno
 '
 
+test_expect_success 'status reports sparse-checkout' '
+	init_repos &&
+	git -C sparse-checkout status >full &&
+	git -C sparse-index status >sparse &&
+	test_i18ngrep "You are in a sparse checkout with " full &&
+	test_i18ngrep "You are in a sparse checkout." sparse
+'
+
 test_expect_success 'add, commit, checkout' '
 	init_repos &&
 
diff --git a/wt-status.c b/wt-status.c
index 0c8287a023e4..0425169c1895 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1490,9 +1490,12 @@  static void show_sparse_checkout_in_use(struct wt_status *s,
 	if (s->state.sparse_checkout_percentage == SPARSE_CHECKOUT_DISABLED)
 		return;
 
-	status_printf_ln(s, color,
-			 _("You are in a sparse checkout with %d%% of tracked files present."),
-			 s->state.sparse_checkout_percentage);
+	if (s->state.sparse_checkout_percentage == SPARSE_CHECKOUT_SPARSE_INDEX)
+		status_printf_ln(s, color, _("You are in a sparse checkout."));
+	else
+		status_printf_ln(s, color,
+				_("You are in a sparse checkout with %d%% of tracked files present."),
+				s->state.sparse_checkout_percentage);
 	wt_longstatus_print_trailer(s);
 }
 
@@ -1650,6 +1653,11 @@  static void wt_status_check_sparse_checkout(struct repository *r,
 		return;
 	}
 
+	if (r->index->sparse_index) {
+		state->sparse_checkout_percentage = SPARSE_CHECKOUT_SPARSE_INDEX;
+		return;
+	}
+
 	for (i = 0; i < r->index->cache_nr; i++) {
 		struct cache_entry *ce = r->index->cache[i];
 		if (ce_skip_worktree(ce))
diff --git a/wt-status.h b/wt-status.h
index 0d32799b28e1..ab9cc9d8f032 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -78,6 +78,7 @@  enum wt_status_format {
 };
 
 #define SPARSE_CHECKOUT_DISABLED -1
+#define SPARSE_CHECKOUT_SPARSE_INDEX -2
 
 struct wt_status_state {
 	int merge_in_progress;