From: Junio C Hamano <gitster@pobox•com>
To: tboegi@web•de
Cc: git@vger•kernel.org
Subject: Re: [PATCH v2 7/7] convert.c: simplify text_stat
Date: Thu, 04 Feb 2016 12:37:03 -0800 [thread overview]
Message-ID: <xmqqh9hogqkw.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <1454608200-5535-1-git-send-email-tboegi@web.de> (tboegi@web.de's message of "Thu, 4 Feb 2016 18:50:00 +0100")
tboegi@web•de writes:
> static int convert_is_binary(unsigned long size, const struct text_stat *stats)
> {
> - if (stats->cr != stats->crlf)
> + if (stats->lonecr)
> return 1;
This is an equivalent conversion, but...
> @@ -266,8 +267,8 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
>
> check_safe_crlf(path, crlf_action, &stats, checksafe);
>
> - /* Optimization: No CR? Nothing to convert, regardless. */
> - if (!stats.cr)
> + /* Optimization: No CRLF? Nothing to convert, regardless. */
> + if (!stats.crlf)
> return 0;
... this is not. In fact this is even better, as we used to try the
remainder of the function when we saw a lone CR, but we no longer
do. I am of course assuming that the original turned into a no-op
if we had a lone CR in the input--otherwise this patch changes the
behaviour.
However, I see this comment after the function returns with the
above optimization:
/*
* If we guessed, we already know we rejected a file with
* lone CR, and we can strip a CR without looking at what
* follow it.
*/
So the code around there used to have a reason to worry about a lone
CR (i.e. it didn't want to lose them). With the change in this hunk
for the "optimization", it no longer is necessary to do so, i.e. we
know we do not have a lone CR so every CR can be stripped because it
must be followed by LF, isn't it?
But I do not see a matching change to simplify that here. Am I
following the current code incorrectly or something?
Puzzled...
> /*
> @@ -315,21 +316,16 @@ static int crlf_to_worktree(const char *path, const char *src, size_t len,
> gather_stats(src, len, &stats);
>
> /* No LF? Nothing to convert, regardless. */
> - if (!stats.lf)
> - return 0;
> -
> - /* Was it already in CRLF format? */
> - if (stats.lf == stats.crlf)
> + if (!stats.lonelf)
Doesn't the comment above need adjustment?
> return 0;
Otherwise, the output side looks correct to me.
next prev parent reply other threads:[~2016-02-04 20:37 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
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 [this message]
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=xmqqh9hogqkw.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