[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    Re: [PATCH 1/5] convert_to_git(): checksafe becomes an integer
From:       Lars Schneider <larsxschneider () gmail ! com>
Date:       2017-12-31 12:23:25
Message-ID: 96B6CD4C-0A0C-47F5-922D-B8BAFB832FD1 () gmail ! com
[Download RAW message or body]


> On 31 Dec 2017, at 09:05, tboegi@web.de wrote:
> 
> From: Torsten Bögershausen <tboegi@web.de>
> 
> When calling convert_to_git(), the checksafe parameter has been used to
> check if commit would give a non-roundtrip conversion of EOL.
> 
> When checksafe was introduced, 3 values had been in use:
> SAFE_CRLF_FALSE: no warning
> SAFE_CRLF_FAIL:  reject the commit if EOL do not roundtrip
> SAFE_CRLF_WARN:  warn the user if EOL do not roundtrip

In general, I really like the direction as this simplifies
my patch later on in 5/5. However, I see a few problems:

(1) The prefix "SAFE_CRLF" confuses me because the main theme
    is EOL and CRLF just happens to be a EOL type.

    What do you think about something like this:
    CONVERT_ERROR_IGNORE     0
    CONVERT_EOL_ERROR_DIE    (1<<0)
    CONVERT_EOL_ERROR_WARN   (1<<1)
    CONVERT_EOL_TO_LF        (1<<2)
    CONVERT_EOL_KEEP_CRLF    (1<<3)
    CONVERT_ENCODE_ERROR_DIE (1<<4)

(2) We mix error reporting switches (FALSE/FAIL/WARN) with
    switches that change the content (RENORMALIZE/KEEP_CRLF).
    Plus, these the switches should be mutually exclusive
    (e.g. we don't want to enable the FAIL and WARN bit at
    the same time).

(3) We kind of replicate some flags defined in cache.h:
    #define HASH_WRITE_OBJECT 1
    #define HASH_RENORMALIZE  4


- Lars


[prev in list] [next in list] [prev in thread] [next in thread]