Hi Usman

Sorry for the slow response

On 31/10/2024 12:21, Usman Akinyemi wrote:

On Thu, Oct 31, 2024 at 9:58 AM Phillip Wood [off-list ref] wrote:

quoted

On 30/10/2024 16:19, Usman Akinyemi wrote:

If you want to work on this that's great, but please don't feel any
obligation to do so.

quoted

I also noticed that the test used for testing used a different
approach(test_must_fail) compared to the one I wrote which used
test_grep. Should I change the test also ?

I'm not sure which test you are looking at but I assume it is using
test_must_fail because the command being tested is expected to die. If
we change the code to print a warning instead then we'd need to capture
stderr and use test_grep or test_cmp. Note that we only want to print a
warning when parsing .gitattributes, the other callers of
parse_whitespace_rule() still want to die. Also we should decide what
value to use when the user provides both - neither indent-with-non-tab
or tab-in-indent are on by default so it's not clear exactly what we
should do.

Hi Philip,

I understand, we will have to pick one if we are to use a warning in this case,
indent-with-non-tab seems to be a good candidate as it is not excluded
by default.

I'm not sure I understand I what you mean by "not excluded by default".

 > We can have something like this>

     if (rule & WS_TAB_IN_INDENT && rule & WS_INDENT_WITH_NON_TAB) {
         warning(_("cannot enforce both tab-in-indent and
indent-with-non-tab, removing tab-in-indent"));
         rule &= ~WS_TAB_IN_INDENT;
     }

That sounds reasonable for the cases where we want to warn rather than die.

and this for default
#define WS_DEFAULT_RULE (WS_TRAILING_SPACE | WS_SPACE_BEFORE_TAB |
WS_INDENT_WITH_NON_TAB | 8)

or just leave the WS_DEFAULT_RULE as it is and remove WS_TAB_IN_INDENT
in case both are set.

I don't think we want to change the default rule as it could cause 
problems for users who rely on it.

Best Wishes

Phillip