tboegi@xxxxxx writes:
> From: Torsten Bögershausen <tboegi@xxxxxx>
>
> Add a helper function to find out, which line endings
> text files should get at checkout, depending on
> core.autocrlf and core.eol
>
> Signed-off-by: Torsten Bögershausen <tboegi@xxxxxx>
> ---
> convert.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/convert.c b/convert.c
> index d0c8c66..9ffd043 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -149,6 +149,19 @@ const char *get_wt_convert_stats_ascii(const char *path)
> return ret;
> }
>
> +static int text_eol_is_crlf(void)
> +{
> + if (auto_crlf == AUTO_CRLF_TRUE)
> + return 1;
> + else if (auto_crlf == AUTO_CRLF_INPUT)
> + return 0;
> + if (core_eol == EOL_CRLF)
> + return 1;
> + if (core_eol == EOL_UNSET && EOL_NATIVE == EOL_CRLF)
> + return 1;
> + return 0;
> +}
> +
> static enum eol output_eol(enum crlf_action crlf_action)
> {
> switch (crlf_action) {
> @@ -164,12 +177,7 @@ static enum eol output_eol(enum crlf_action crlf_action)
> /* fall through */
> case CRLF_TEXT:
> case CRLF_AUTO:
> - if (auto_crlf == AUTO_CRLF_TRUE)
> - return EOL_CRLF;
> - else if (auto_crlf == AUTO_CRLF_INPUT)
> - return EOL_LF;
> - else if (core_eol == EOL_UNSET)
> - return EOL_NATIVE;
> + return text_eol_is_crlf() ? EOL_CRLF : EOL_LF;
> }
> return core_eol;
> }
> @@ -1378,8 +1386,9 @@ struct stream_filter *get_stream_filter(const char *path, const unsigned char *s
> (crlf_action == CRLF_GUESS && auto_crlf == AUTO_CRLF_FALSE))
> filter = cascade_filter(filter, &null_filter_singleton);
>
> - else if (output_eol(crlf_action) == EOL_CRLF &&
> - !(crlf_action == CRLF_AUTO || crlf_action == CRLF_GUESS))
> + else if ((crlf_action == CRLF_AUTO || crlf_action == CRLF_GUESS))
> + ;
> + else if (output_eol(crlf_action) == EOL_CRLF)
> filter = cascade_filter(filter, lf_to_crlf_filter());
>
> return filter;
I am a bit slow today so let me talk this change through aloud.
The condition under which we called cascade_filter() used to be that
output_eol(crlf_action) yields EOL_CRLF and crlf_action is not set
to one of these two values. Now, we say if crlf_action is one of
these two values, we wouldn't call it, and then we check
output_eol().
So they do the same thing, even though it smells that this change is
outside the scope of "Add a helper and use it" theme of the patch.
While I do not think this new "split" conditions particularly easier
to read compared to the previous one, if you plan to do something
different in later patches when crlf_action is set to specific
values, ignoring what output_eol() says, a patch to implement such a
change would become easier to understand what is going on with this
preparatory rewrite. So if such a preparatory rewrite is coming (I
haven't checked 5-7/7 yet), perhaps have this hunk as a single patch
that is separate from "add a helper text_eol_is_crlf()" patch.
By the way, your new 1/7 has s/: Remove/: remove/ applied to the
subject, but not other ones like this one. Intended, or you forgot
to do s/: Use/: use/ here?
--
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