Re: [PATCH 2/3] var: add attributes files locations

From: Derrick Stolee <derrickstolee@github.com>
Date: Jun 22, 2023, 20:19 UTC
Message-ID: <ebdf8a20-9ac7-dcf7-8cc6-b63894b7eb45@github.com>
In-Reply-To: <20230622195059.320593-3-sandals@crustytoothpaste.net>

On 6/22/2023 3:50 PM, brian m. carlson wrote:

Show 5 quoted lines

>  struct git_var {
>  	const char *name;
>  	const char *(*read)(int);
> +	int free;
>  };

I see you're expanding the git_var struct, and you do it more in patch 3. At that point, there will be two consecutive 'int' parameters, which can make things unclear as to how to proceed.

Show 10 quoted lines

>  static struct git_var git_vars[] = {
> -	{ "GIT_COMMITTER_IDENT", git_committer_info },
> -	{ "GIT_AUTHOR_IDENT",   git_author_info },
> -	{ "GIT_EDITOR", editor },
> -	{ "GIT_SEQUENCE_EDITOR", sequence_editor },
> -	{ "GIT_PAGER", pager },
> -	{ "GIT_DEFAULT_BRANCH", default_branch },
> -	{ "GIT_SHELL_PATH", shell_path },
> +	{ "GIT_COMMITTER_IDENT", git_committer_info, 0 },
> +	{ "GIT_AUTHOR_IDENT",   git_author_info, 0 },

This also makes for a big diff like this.

One way to solve for this is to use the more modern style when initializing the structs:

static struct git_var git_vars[] = {
	{
		.name = "GIT_COMMITTER_IDENT",
		.read = git_author_info,
		.free = 0,
	},
	...
}

Bonus points if you do this conversion before modifying the struct, as later changes will only add lines to the existing initialization...

  static struct git_var git_vars[] = {
  	{
  		.name = "GIT_COMMITTER_IDENT",
  		.read = git_author_info,
+ 		.multivalued = 0,
  		.free = 0,
  	},
  	...
  }

Thanks, -Stolee