[sane-devel] [janitorial] leading whitespace: spaces XOR tabs

Olaf Meeuwissen paddy-hack at member.fsf.org
Thu Jul 20 11:49:42 UTC 2017


Hi all,

Apologies for replying to my own, original post but I want to reply to
all initial follow-up in a single post.

First off, Gerhard, Allan, Johannes, many thanks for the feedback.

Your summarized comments with my replies first, followed by in-line
comments on my initial post.

 1. The whole code base?  Is that worth it?

    In principle, yes, the whole code base unless backend maintainers
    balk.  Whether it's worth it, I don't know.  I only know that at
    least I will feel better (but that's not all important, of course).
    As a self-appointed janitor I get to deal with a larger part of the
    code base than the average backend maintainer.  Switching between
    space/tab conventions on a per line basis and indenting and coding
    styles between blocks (at best!) sometimes feels like a juggling
    job.
    # And my hand-eye coordination leaves to be desired ;-)

 2. How about unmaintained backends only?

    That might be an option but please note that the maintained status
    was decided based on "backend contributor mentioned in the AUTHORS
    file with commit privileges" without regard for actual status.

    @Gerhard> Are you still *maintaining* the plustek and plustek-pp
              backends?  If yes, how about you clean up ;-)?

 3. Wouldn't that break people's patches?

    Get them upstreamed.  Seriously, what's keeping everyone's generally
    usable private patches from getting included?
    In case your patches don't make it in for some reason, have a good
    look at the documentation for the --ignore-whitespace options.  Both
    git am and git apply as well as the patch program support it.

 4. But that makes git blame harder to use!

    git blame is only really useful if your UI let's you quickly jump to
    the changeset in question and move to the one before it with ease.
    If it doesn't, Johannes' git log --follow -p is magnitudes better.
    # BTW, I use both approaches, judiciously.

 5. Whitespace issues haven't really bothered me.

    That's you.  They do bother *me*.  That's why I brought this up in
    the first place ;-)
    Clean, consistently formatted code is a lot easier and inviting to
    work with.  A SANE approach to leading whitespace at the file level
    is a start.  Pun intended.
    As you mentioned, such code may make it easier for folks to pick up
    the maintenance of a backend.  If so, we're off for the better.
    # As a "janitor", I'm just trying to fix "broken windows" and patch
    # up "cracks in the walls" of the sane-backends apartment building.

 6. Spaces-only leads to more intelligible diffs.

    ACK.  Less weird indenting when looking at the diff.

 7. Spaces everywhere please, I want to get rich quick ;-)

    Let's combine that with a one-space indent style so we can inject a
    pyramid scheme and get rich quicker yet :-P

And now for some observations pouring over my inital post and the
current state of white space use.

Olaf Meeuwissen writes:

> [... fixing "hair-raising" -Wmisleading-indentation issues ...]
>
> This whole exercise has made me look at the whole code base in a little
> more detail and, quite frankly, the use of leading whitespace is a total
> and utter mess.  Some places are better of than others but on the whole
> it's pretty pathetic.

OK, so that may have been a bit over the top, but, hey, I want to grab
everyone's attention.

> So here's a few "rules" I'd like to apply in order to address this.
> Each file
>
>  - uses either spaces or tabs for *all* of its leading whitespace
>    This is *not* on a per line basis, this applies to the *whole* file.
>  - if using tabs, these tabs may be followed by up to 7 spaces
>    This is to accommodate n-space indent policies (where n mod 8 != 0)
>    and assumes eight spaces to the tab.
>  - if using tabs, uses eight spaces to the tab
>  - if to be used by `make`, an initial whitespace character shall be
>    a tab, independent of whether its on a continuation line or not

Let's add ChangeLogs/ChangeLog-1.0.$n for n < 26 to the files that have
a leading tab.  Most of them are pretty close to that already.

I've written (and attached) a little script that analyses every file you
throw at it following something close to the above.  Please have a look
at the script to get an idea of what is does and what the output is all
about.

Suggested use:

  git ls-files | xargs -n 1 tabs-and-spaces.sh

> The "rules" above are inspired by a combination of consistency, ease of
> checking/fixing and giving developers some leeway in applying their own
> preferences to their code.

It turns out that the checking is the easy part.  The fixing can be a
pain in the behind and may require a fair bit of manual intervention :-(

> Whether to use spaces or tabs for each file will be decided on the basis
> of a count of tabs and spaces (and in case of doubt comparison with any
> related files so as to keep some kind of consistency between them).  The
> criterion will be a majority of one over the other (taking into account
> the number of spaces to a tab).  So, with n spaces to the tab, the
> larger value of n*number_of_leading_tabs and number_of_leading_spaces
> will decide the leading whitespace policy for a file.

This turns out to be not such a clear cut issue.  I'll need to study the
output of my script in a more detail and perhaps apply some case-by-case
logic.

> If I hear nothing, the tools/style-check.sh script will be modified to
> implement these rules and can then be use to check for any "violations"
> and (recursively) fix them.

I did hear something, so I'll think things over a bit more and may end
up doing something in a branch so that we can all take a look and decide
what's for the better.

Hope this helps,
--
Olaf Meeuwissen, LPIC-2            FSF Associate Member since 2004-01-27
 GnuPG key: F84A2DD9/B3C0 2F47 EA19 64F4 9F13  F43E B8A4 A88A F84A 2DD9
 Support Free Software                        https://my.fsf.org/donate
 Join the Free Software Foundation              https://my.fsf.org/join
-------------- next part --------------
A non-text attachment was scrubbed...
Name: tabs-and-spaces.sh
Type: text/x-sh
Size: 1995 bytes
Desc: Tabs/spaces analysis tool
URL: <http://lists.alioth.debian.org/pipermail/sane-devel/attachments/20170720/cdecda5f/attachment.sh>


More information about the sane-devel mailing list