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 <phillip.wood123@xxxxxxxxx> wrote:
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.
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
