From: Junio C Hamano <gitster@pobox•com>
To: tboegi@web•de
Cc: git@vger•kernel.org
Subject: Re: [PATCH v1 3/6] convert.c: Remove input_crlf_action()
Date: Tue, 02 Feb 2016 13:44:46 -0800 [thread overview]
Message-ID: <xmqqbn7yrdm9.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <1454431992-2019-1-git-send-email-tboegi@web.de> (tboegi@web.de's message of "Tue, 2 Feb 2016 17:53:12 +0100")
tboegi@web•de writes:
> From: Torsten Bögershausen <tboegi@web•de>
>
> Integrate the code of input_crlf_action() into convert_attrs(),
> so that ca.crlf_action is aleays valid after calling convert_attrs().
s/aleays/always/
> Keep a copy of crlf_action in attr_action, this is needed for
> get_convert_attr_ascii().
>
> Remove eol_attr from struct conv_attrs, as it is now used temporally.
>
> Signed-off-by: Torsten Bögershausen <tboegi@web•de>
> ---
> convert.c | 35 +++++++++++++----------------------
> 1 file changed, 13 insertions(+), 22 deletions(-)
>
> diff --git a/convert.c b/convert.c
> index a24c2a2..bca3d0c 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -746,21 +746,10 @@ static int git_path_check_ident(struct git_attr_check *check)
> return !!ATTR_TRUE(value);
> }
>
> -static enum crlf_action input_crlf_action(enum crlf_action text_attr, enum eol eol_attr)
> -{
> - if (text_attr == CRLF_BINARY)
> - return CRLF_BINARY;
> - if (eol_attr == EOL_LF)
> - return CRLF_INPUT;
> - if (eol_attr == EOL_CRLF)
> - return CRLF_CRLF;
> - return text_attr;
> -}
> -
> struct conv_attrs {
> struct convert_driver *drv;
> - enum crlf_action crlf_action;
> - enum eol eol_attr;
> + enum crlf_action attr_action; /* What attr says */
> + enum crlf_action crlf_action; /* When no attr is set, use core.autocrlf */
> int ident;
> };
>
> @@ -782,16 +771,23 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)
> }
>
> if (!git_check_attr(path, NUM_CONV_ATTRS, ccheck)) {
> + enum eol eol_attr;
> ca->crlf_action = git_path_check_crlf( ccheck + 4);
I guess this extra SP after opening paren was a misconversion from
the previous step...
> if (ca->crlf_action == CRLF_GUESS)
> ca->crlf_action = git_path_check_crlf(ccheck + 0);
> + ca->attr_action = ca->crlf_action;
> 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);
> + if (ca->crlf_action == CRLF_BINARY)
> + return;
> + eol_attr = git_path_check_eol(ccheck + 3);
> + if (eol_attr == EOL_LF)
> + ca->crlf_action = CRLF_INPUT;
> + else if (eol_attr == EOL_CRLF)
> + ca->crlf_action = CRLF_CRLF;
> } else {
> ca->drv = NULL;
> ca->crlf_action = CRLF_GUESS;
> - ca->eol_attr = EOL_UNSET;
> ca->ident = 0;
> }
> }
> @@ -818,11 +814,9 @@ int would_convert_to_git_filter_fd(const char *path)
> const char *get_convert_attr_ascii(const char *path)
> {
> struct conv_attrs ca;
> - enum crlf_action crlf_action;
>
> convert_attrs(&ca, path);
> - crlf_action = input_crlf_action(ca.crlf_action, ca.eol_attr);
> - switch (crlf_action) {
> + switch (ca.attr_action) {
> case CRLF_GUESS:
> return "";
> case CRLF_BINARY:
> @@ -861,7 +855,6 @@ int convert_to_git(const char *path, const char *src, size_t len,
> src = dst->buf;
> len = dst->len;
> }
> - ca.crlf_action = input_crlf_action(ca.crlf_action, ca.eol_attr);
> ret |= crlf_to_git(path, src, len, dst, ca.crlf_action, checksafe);
> if (ret && dst) {
> src = dst->buf;
> @@ -882,7 +875,6 @@ void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst,
> if (!apply_filter(path, NULL, 0, fd, dst, ca.drv->clean))
> die("%s: clean filter '%s' failed", path, ca.drv->name);
>
> - ca.crlf_action = input_crlf_action(ca.crlf_action, ca.eol_attr);
> crlf_to_git(path, dst->buf, dst->len, dst, ca.crlf_action, checksafe);
> ident_to_git(path, dst->buf, dst->len, dst, ca.ident);
> }
> @@ -912,7 +904,6 @@ static int convert_to_working_tree_internal(const char *path, const char *src,
> * is a smudge filter. The filter might expect CRLFs.
> */
> if (filter || !normalizing) {
> - ca.crlf_action = input_crlf_action(ca.crlf_action, ca.eol_attr);
> ret |= crlf_to_worktree(path, src, len, dst, ca.crlf_action);
> if (ret) {
> src = dst->buf;
> @@ -1381,7 +1372,7 @@ struct stream_filter *get_stream_filter(const char *path, const unsigned char *s
> if (ca.ident)
> filter = ident_filter(sha1);
>
> - crlf_action = input_crlf_action(ca.crlf_action, ca.eol_attr);
> + crlf_action = ca.crlf_action;
>
> if ((crlf_action == CRLF_BINARY) || (crlf_action == CRLF_INPUT) ||
> (crlf_action == CRLF_GUESS && auto_crlf == AUTO_CRLF_FALSE))
So the idea is that the code before this prepared eol_attr and
crlf_action fields in convert_attrs() and consumers of these fields
called input_crlf_action() with them as parameters to choose one of
CRLF_X, but with this change, that is precomputed and left in the
new field crlf_action.
I didn't find a code where ca.eol_attr is redefined after
convert_attrs() returned, and all the codepaths that redefine
ca.crlf_action do so by assigning the result of the call to
input_crlf_action(), _and_ they do not use the value before doing
so. So it appears that this should not change any behaviour.
OK.
next prev parent reply other threads:[~2016-02-02 21:44 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <Message-Id=1453558101-6858-1-git-send-email-tboegi@web.de>
2016-01-24 7:55 ` [PATCH v2] t0027: Add tests for get_stream_filter() tboegi
2016-01-27 6:34 ` Junio C Hamano
2016-01-27 9:05 ` Torsten Bögershausen
2016-01-27 15:15 ` [PATCH v1 1/6] " tboegi
2016-02-02 16:53 ` tboegi
2016-02-02 21:18 ` Junio C Hamano
2016-02-02 16:53 ` [PATCH v1 2/6] convert.c: Remove path when not needed tboegi
2016-02-02 21:32 ` Junio C Hamano
2016-02-02 16:53 ` [PATCH v1 3/6] convert.c: Remove input_crlf_action() tboegi
2016-02-02 21:44 ` Junio C Hamano [this message]
2016-02-02 16:53 ` [PATCH v1 4/6] convert.c: Use text_eol_is_crlf() tboegi
2016-02-02 16:53 ` [PATCH v1 5/6] convert: auto_crlf=false and no attributes set: same as binary tboegi
2016-02-02 16:53 ` [PATCH v1 6/6] convert.c: Refactor crlf_action tboegi
2016-02-04 17:49 ` [PATCH v2 1/7] t0027: Add tests for get_stream_filter() tboegi
2016-02-04 19:52 ` Junio C Hamano
2016-02-04 17:49 ` [PATCH v2 2/7] convert.c: remove unused parameter 'path' tboegi
2016-02-04 17:49 ` [PATCH v2 3/7] convert.c: Remove input_crlf_action() tboegi
2016-02-04 17:49 ` [PATCH v2 4/7] convert.c: Use text_eol_is_crlf() tboegi
2016-02-04 20:13 ` Junio C Hamano
2016-02-04 17:49 ` [PATCH v2 5/7] convert: auto_crlf=false and no attributes set: same as binary tboegi
2016-02-04 17:49 ` [PATCH v2 6/7] convert.c: Refactor crlf_action tboegi
2016-02-04 17:50 ` [PATCH v2 7/7] convert.c: simplify text_stat tboegi
2016-02-04 20:37 ` Junio C Hamano
2016-02-05 16:13 ` [PATCH v3 1/7] t0027: Add tests for get_stream_filter() tboegi
2016-02-08 17:59 ` Junio C Hamano
2016-02-05 16:13 ` [PATCH v3 2/7] convert.c: remove unused parameter 'path' tboegi
2016-02-05 16:13 ` [PATCH v3 3/7] convert.c: Remove input_crlf_action() tboegi
2016-02-05 16:13 ` [PATCH v3 4/7] convert.c: use text_eol_is_crlf() tboegi
2016-02-05 16:13 ` [PATCH v3 5/7] convert: auto_crlf=false and no attributes set: same as binary tboegi
2016-02-08 18:27 ` Junio C Hamano
2016-02-09 14:34 ` Torsten Bögershausen
2016-02-09 18:06 ` Junio C Hamano
2016-02-05 16:13 ` [PATCH v3 6/7] convert.c: refactor crlf_action tboegi
2016-02-05 16:13 ` [PATCH v3 7/7] convert.c: simplify text_stat tboegi
2016-02-10 16:24 ` [PATCH v4 1/6] t0027: add tests for get_stream_filter() tboegi
2016-02-10 16:24 ` [PATCH v4 2/6] convert.c: remove unused parameter 'path' tboegi
2016-02-10 16:24 ` [PATCH v4 3/6] convert.c: remove input_crlf_action() tboegi
2016-02-10 16:24 ` [PATCH v4 4/6] convert.c: use text_eol_is_crlf() tboegi
2016-02-10 16:24 ` [PATCH v4 5/6] convert.c: refactor crlf_action tboegi
2016-02-10 16:24 ` [PATCH v4 6/6] convert.c: simplify text_stat tboegi
2016-02-22 5:11 ` [PATCH 1/1] convert.c: correct attr_action tboegi
2016-02-22 5:34 ` Eric Sunshine
2016-02-22 8:04 ` Junio C Hamano
2016-02-22 8:20 ` Junio C Hamano
2016-02-23 5:26 ` Torsten Bögershausen
2016-02-23 17:07 ` [PATCH v2 " tboegi
2016-02-23 20:52 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqbn7yrdm9.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=tboegi@web$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox