tboegi@xxxxxx writes:
> diff --git a/convert.c b/convert.c
> index 18af685..0bc32ec 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -708,7 +708,7 @@ static enum crlf_action git_path_check_crlf(struct git_attr_check *check)
> const char *value = check->value;
>
> if (ATTR_TRUE(value))
> - return text_eol_is_crlf() ? CRLF_TEXT_CRLF : CRLF_TEXT_INPUT;
> + return CRLF_TEXT;
> else if (ATTR_FALSE(value))
> return CRLF_BINARY;
> else if (ATTR_UNSET(value))
Hmph, this function has exactly two callers, and they call it like
this:
if (!git_check_attr(path, NUM_CONV_ATTRS, ccheck)) {
enum eol eol_attr;
ca->crlf_action = git_path_check_crlf(ccheck + 4);
if (ca->crlf_action == CRLF_UNDEFINED)
ca->crlf_action = git_path_check_crlf(ccheck + 0);
ca->attr_action = ca->crlf_action;
where ccheck+0 refers to "crlf" and ccheck+4 refers to "text".
So, we say "first check 'text' attribute, and if it is true we say
CRLF_TEXT and stop." We also say "first check 'text', and if it is
not there, the first call returns undef, in which case we check 'crlf'
attribute, and if it is true we say CRLF_TEXT".
And ca->attr_action retains this value.
However, immediately after this assignment to ca->attr_action, we
have this:
ca->ident = git_path_check_ident(ccheck + 1);
ca->drv = git_path_check_convert(ccheck + 2);
if (ca->crlf_action == CRLF_BINARY)
return;
eol_attr = git_path_check_eol(ccheck + 3);
if (eol_attr == EOL_LF)
ca->crlf_action = CRLF_TEXT_INPUT;
else if (eol_attr == EOL_CRLF)
ca->crlf_action = CRLF_TEXT_CRLF;
ca->attr_action = ca->crlf_action;
We check ident (ccheck+1) and filter (ccheck+2); if the value of
ca->crlf_action (which was determined by checking 'text' and then
'crlf') is not CRLF_BINARY, we further check eol (ccheck+3) and
update ca->crlf_action based on it. And ca->attr_action gets
updated again.
This feels unnecessarily convoluted. Perhaps if you get rid of the
early return for CRLF_BINARY, the flow may become a lot clearer,
i.e.
ca->crlf_action = check 'text';
if (ca->crlf_action == undef)
ca->crlf_action = check 'crlf';
// DO NOT ASSIGN ca->attr_action HERE YET
ca->ident = check 'ident';
ca->drv = check 'filter';
if (crlf_action != CRLF_BINARY) {
eol_attr = check 'eol';
if (eol == LF)
ca->crlf_action = ...
else if (eol == EOL_CRLF)
ca->crlf_action = ...
}
ca->attr_action = ca->crlf_action;
Hmm?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html