On Thu, May 15, 2025 at 01:37:00PM -0700, Junio C Hamano wrote:
> Jeff King <peff@xxxxxxxx> writes:
>
> > ... there are any non-ita entries). Though in that case I'd think:
> >
> > committable = 0;
> > for (i = 0; i < cache_nr; i++) {
> > if (!ce_intent_to_add(...) {
> > committable = 1;
> > break;
> > }
> > }
> >
> > would be the most clear, since we do not otherwise care about the actual
> > number of ita entries. And lets us break out of the loop early.
>
> Exactly. If you focus on the warning too narrowly, the minimal
> change in the original patch does look OK, but in the original (even
> before Dscho's patch, that is) the intent is unclear, as opposed to
> what you showed above. And the update to squelch false positive
> does not improve the clarity of the logic as the above rewrite does.
OK. If we do want to refactor, I think pulling it into a separate
function is the most descriptive, like:
diff --git a/builtin/commit.c b/builtin/commit.c
index 66bd91fd52..a8d43d223d 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -740,6 +740,15 @@ static void change_data_free(void *util, const char *str UNUSED)
free(d);
}
+static int has_non_ita_entries(struct index_state *index)
+{
+ int i;
+ for (i = 0; i < index->cache_nr; i++)
+ if (!ce_intent_to_add(index->cache[i]))
+ return 1;
+ return 0;
+}
+
static int prepare_to_commit(const char *index_file, const char *prefix,
struct commit *current_head,
struct wt_status *s,
@@ -1015,14 +1024,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
parent = "HEAD^1";
if (repo_get_oid(the_repository, parent, &oid)) {
- int i, ita_nr = 0;
-
/* TODO: audit for interaction with sparse-index. */
ensure_full_index(the_repository->index);
- for (i = 0; i < the_repository->index->cache_nr; i++)
- if (ce_intent_to_add(the_repository->index->cache[i]))
- ita_nr++;
- committable = the_repository->index->cache_nr - ita_nr > 0;
+ committable =
+ has_non_ita_entries(the_repository->index);
} else {
/*
* Unless the user did explicitly request a submodule
-Peff
