Re: [PATCH 2/3] Define 'crlf' attribute.
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Apr 15, 2007, 15:54 UTC
- Message-ID
- <Pine.LNX.4.64.0704151141090.18846@racer.site>
- In-Reply-To
- <7vps68d4wf.fsf@assigned-by-dhcp.cox.net>
Hi,
On Fri, 13 Apr 2007, Junio C Hamano wrote:
Show 14 quoted lines
> diff --git a/attr.c b/attr.c
> index bdbc4a3..d35ae9e 100644
> --- a/attr.c
> +++ b/attr.c
> [...]
> +
> +int git_path_is_binary(const char *path)
> +{
> + struct git_attr_check attr_binary_check;
> +
> + setup_binary_check(&attr_binary_check);
> + return (!git_checkattr(path, 1, &attr_binary_check) &&
> + (0 < attr_binary_check.isset));
> +}This function is not declared in a header file, and ...
Show 20 quoted lines
> diff --git a/convert.c b/convert.c
> index 898bfe3..20c744a 100644
> --- a/convert.c
> +++ b/convert.c
> [...]
> +
> +static int git_path_is_binary(const char *path)
> +{
> + struct git_attr_check attr_crlf_check;
> +
> + setup_crlf_check(&attr_crlf_check);
> +
> + /*
> + * If crlf is not mentioned, default to autocrlf;
> + * disable autocrlf only when crlf attribute is explicitly
> + * unset.
> + */
> + return (!git_checkattr(path, 1, &attr_crlf_check) &&
> + (0 == attr_crlf_check.isset));
> +}... here you have a function of the _same_ name, which does something subtly different: it checks for "crlf" instead of "binary".
Perhaps you wanted the change to attr.c at a later stage, and the function in convert.c should be renamed to "path_wants_crlf()"?
[Reading 3/3 revealed that indeed, the 18 lines in attr.c were unwanted.]
Ciao, Dscho