[email protected] writes: > From: Torsten Bögershausen <[email protected]> > > 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 <[email protected]> > --- > 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
