Re: [PATCH v7 2/7] submodule: strbuf variable rename

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Feb 7, 2023, 22:47 UTC
Message-ID: <230207.86ttzx13xc.gmgdl@evledraar.gmail.com>
In-Reply-To: <20230207181706.363453-3-calvinwan@google.com>

On Tue, Feb 07 2023, Calvin Wan wrote:

> A prepatory change for a future patch that moves the status parsing
> logic to a separate function.

Ah, I think I suggested splitting this up in some previous round, and coming back to this this + the next patch look very nice with the move detection, thanks!

Show 9 quoted lines

>  	fp = xfdopen(cp.out, "r");
>  	while (strbuf_getwholeline(&buf, fp, '\n') != EOF) {
> +		char *str = buf.buf;
> +		const size_t len = buf.len;
> +
>  		/* regular untracked files */
> -		if (buf.buf[0] == '?')
> +		if (str[0] == '?')
>  			dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED;

I'll only add that we could also do this on top:
	
	diff --git a/submodule.c b/submodule.c
	index c7c6bfb2e26..eeb940d96a0 100644
	--- a/submodule.c
	+++ b/submodule.c
	@@ -1875,7 +1875,7 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)
	 	struct child_process cp = CHILD_PROCESS_INIT;
	 	struct strbuf buf = STRBUF_INIT;
	 	FILE *fp;
	-	unsigned dirty_submodule = 0;
	+	unsigned dirty_submodule0 = 0;
	 	const char *git_dir;
	 	int ignore_cp_exit_code = 0;
	 
	@@ -1908,10 +1908,11 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)
	 	while (strbuf_getwholeline(&buf, fp, '\n') != EOF) {
	 		char *str = buf.buf;
	 		const size_t len = buf.len;
	+		unsigned *dirty_submodule = &dirty_submodule0;
	 
	 		/* regular untracked files */
	 		if (str[0] == '?')
	-			dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED;
	+			*dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED;
	 
	 		if (str[0] == 'u' ||
	 		    str[0] == '1' ||
	@@ -1923,17 +1924,17 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)
	 
	 			if (str[5] == 'S' && str[8] == 'U')
	 				/* nested untracked file */
	-				dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED;
	+				*dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED;
	 
	 			if (str[0] == 'u' ||
	 			    str[0] == '2' ||
	 			    memcmp(str + 5, "S..U", 4))
	 				/* other change */
	-				dirty_submodule |= DIRTY_SUBMODULE_MODIFIED;
	+				*dirty_submodule |= DIRTY_SUBMODULE_MODIFIED;
	 		}
	 
	-		if ((dirty_submodule & DIRTY_SUBMODULE_MODIFIED) &&
	-		    ((dirty_submodule & DIRTY_SUBMODULE_UNTRACKED) ||
	+		if ((*dirty_submodule & DIRTY_SUBMODULE_MODIFIED) &&
	+		    ((*dirty_submodule & DIRTY_SUBMODULE_UNTRACKED) ||
	 		     ignore_untracked)) {
	 			/*
	 			 * We're not interested in any further information from
	@@ -1949,7 +1950,7 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)
	 		die(_("'git status --porcelain=2' failed in submodule %s"), path);
	 
	 	strbuf_release(&buf);
	-	return dirty_submodule;
	+	return dirty_submodule0;
	 }
	 
	 int submodule_uses_gitfile(const char *path)

Which, if we're massaging this for a subsequent smaller diff we can do to make only the comment adjustment part of this be a non-moved line.