Re: [PATCH v1 2/6] convert.c: Remove path when not needed

[Junio C Hamano <gitster@pobox.com>]

tboegi@web.de writes:


> Subject: Re: [PATCH v1 2/6] convert.c: Remove path when not needed

At least s/: Remove/: remove/, but I think

s/: Remove.*/: remove unused parameter 'path'/

would be easier to read.

All of these functions seem to have "path" in their name, sounding
as if they meant to ask "What is the crlf-ness for THIS PATH?"  Is
it still sensible to have "path" in their names?

The necessary information that is specific to the path was already
gathered when we prepared the 'check' thing, so they are still
asking and answering their questions specific to the path by
accepting 'check', but it still feels somewhat funny.

With or without "path" in their names, they are horribly misnamed
(e.g. "check crlf---are we asking if the file has crlf?  if the
file must be written out with crlf?  something else?").

Your later patches seem to refactor and reorganize this internal
calling mess in some way, so let's read on and see how the whole
thing improves ;-)

> -static enum crlf_action git_path_check_crlf(const char *path, struct git_attr_check *check)
> +static enum crlf_action git_path_check_crlf(struct git_attr_check *check)
>  {
>  	const char *value = check->value;
>  
> @@ -713,7 +713,7 @@ static enum crlf_action git_path_check_crlf(const char *path, struct git_attr_ch
>  	return CRLF_GUESS;
>  }
>  
> -static enum eol git_path_check_eol(const char *path, struct git_attr_check *check)
> +static enum eol git_path_check_eol(struct git_attr_check *check)
>  {
>  	const char *value = check->value;
>  
> @@ -726,8 +726,7 @@ static enum eol git_path_check_eol(const char *path, struct git_attr_check *chec
>  	return EOL_UNSET;
>  }
>  
> -static struct convert_driver *git_path_check_convert(const char *path,
> -					     struct git_attr_check *check)
> +static struct convert_driver *git_path_check_convert(struct git_attr_check *check)
>  {
>  	const char *value = check->value;
>  	struct convert_driver *drv;
> @@ -740,7 +739,7 @@ static struct convert_driver *git_path_check_convert(const char *path,
>  	return NULL;
>  }
>  
> -static int git_path_check_ident(const char *path, struct git_attr_check *check)
> +static int git_path_check_ident(struct git_attr_check *check)
>  {
>  	const char *value = check->value;
>  
> @@ -783,12 +782,12 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)
>  	}
>  
>  	if (!git_check_attr(path, NUM_CONV_ATTRS, ccheck)) {
> -		ca->crlf_action = git_path_check_crlf(path, ccheck + 4);
> +		ca->crlf_action = git_path_check_crlf( ccheck + 4);
>  		if (ca->crlf_action == CRLF_GUESS)
> -			ca->crlf_action = git_path_check_crlf(path, ccheck + 0);
> -		ca->ident = git_path_check_ident(path, ccheck + 1);
> -		ca->drv = git_path_check_convert(path, ccheck + 2);
> -		ca->eol_attr = git_path_check_eol(path, ccheck + 3);
> +			ca->crlf_action = git_path_check_crlf(ccheck + 0);
> +		ca->ident = git_path_check_ident(ccheck + 1);
> +		ca->drv = git_path_check_convert(ccheck + 2);
> +		ca->eol_attr = git_path_check_eol(ccheck + 3);
>  	} else {
>  		ca->drv = NULL;
>  		ca->crlf_action = CRLF_GUESS;