Torsten Bögershausen <[email protected]> writes:
>> * convert_attrs() has "If BINARY don't do anything and return".
>> Will the patch change behaviour for the "not-autocrlf,
>> CRLF_GUESS" case in this codepath? I think ca->crlf_action used
>> to be left as CRLF_GUESS here before the patch, and now by the
>> time the control flow reaches here it is already CRLF_BINARY.
>> Would it affect the callers, and if so how?
> Not sure if I fully understand the question:
>
> The old CRLF_GUESS could mean (a) core.autocrlf=true,
> (b) core.autocrlf=input or (c) core.autocrlf=false.
> The callers had to look at the core.autocrlf them self.
> This patch removes (c), the next (or over next) (a) and (b)
That part I understand and fully agree.
If a function that appears much later in the control flow depends
on seeing CRLF_GUESS to do certain things, and showing CRLF_BINARY
to the code would make it behave differently, that would make this
patch incorrect.
That is, if some function that you did not touch (hence did not
appear in the context of this patch) did this:
if (CRLF_GUESS)
do the "guess thing"
else if (CRLF_BINARY)
do the "binary thing"
else
do other things
then depending on how 'guess thing' and 'binary thing' work
differently, this patch would change the outcome. If the
"guess thing" had
if (!AUTOCRLF) goto "binary thing"
upfront, then it obviously is OK, though ;-)
If the _ONLY_ way all the code that use CRLF_GUESS is
if ((CRLF_GUESS && !AUTOCRLF) || CRLF_BINARY)
do the "binary thing"
then the conversion done by the patch is perfectly fine. It was
unclear from the patch text and the proposed log message if you made
sure that is the case. And with your response, it still isn't clear
to me.
--
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
