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

[Derrick Stolee <derrickstolee@xxxxxxxxxx>]

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

>  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.

>  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