[PATCH] Add utility to check for indentation in files or patches

Simo simo at samba.org
Thu Apr 12 15:25:50 UTC 2018


On Tue, 2018-04-10 at 10:27 +1000, Martin Schwenke via samba-technical
wrote:
> Hi Simo,
> 
> On Mon, 09 Apr 2018 18:18:11 -0400, Simo <simo at samba.org> wrote:
> 
> > Attached find a new version that deals with python3 properly and
> > also has better handling of indent, by spitting only a diff file with
> > the propsed diff you should apply to pass it's rules.
> > Previously the code was trying to line match and give a generic error
> > with the line number but that doesn't work because indent can change
> > lines by wrapping or unwrapping so the line numbers would make no sense
> >  on longer files.
> 
> Looks good!  I notice that it doesn't exit with non-zero when it
> (thinks that it) finds issues in my current branch.  Do we want it to
> do that?  Would that be as simple as having warn() set a global?

No I think we want to exit with an error when it failed to check stuff,
a commit hook would rather decide based on the output ?
But we can change that later once we actually propose a hook if we find
it better to deal with it via return errors.

> > I have no yet addressed the space and parenthesized issues in this
> > specific email simply because I ran out of time and I am not sure we
> > actually do not want them.
> > 
> > I am open to changing these though if more people agree on the need for
> > changing them.
> 
> I don't think that we should error on things that README.Coding doesn't
> prohibit, particularly if they appear in "good" examples.  I think this
> is particularly true if people are pushing hard for this code to be
> used in tests to validate code.  It would be wrong to have tests that
> contradict README.Coding.  We could, of course, change README.Coding if
> there is a good reason to do so.

It is not easy to figure out exactly what is required, we also suggest
using indent, but I already had to add one more indent option to the
ones we mention there to account for the fact we keep labels on the
beginning of the line and do not indent them.

> I hope that others agree...  :-)

I would definitely like someone else to add feedback or RB+

> I note that these particular checks seem to be implemented in
> cstyle-file.py (via check_binary_operator() and
> check_parenthesized_return()), so they should be easy to remove.

I think we could perhaps gather them into an optional step, but the
current code does not allow for adding options.
I can disable them if someone else wants to comment.

Wouldn't want to go back and forth too many times.

Simo.



More information about the samba-technical mailing list