Project

General

Profile

Bug #14246

Inconsistent C source code indentation

Added by shyouhei (Shyouhei Urabe) 28 days ago. Updated about 1 hour ago.

Status:
Open
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:84498]

Description

I would like to focus on following 238 C source codes in our repository.

% git ls-files | grep '\.\(c\|h\|def\)$' | grep -v 'ext|spec|test' | wc -l
238

Here, in these 238 files, 10 files are indented using spaces only.

% git ls-files | grep '\.\(c\|h\|def\)$' | grep -v 'ext|spec|test' | \
xargs grep --files-without-match $'^\t' | xargs grep --files-with-match '^        ' | wc -l
10

On the other hand 66 files are indented using tabs.

% git ls-files | grep '\.\(c\|h\|def\)$' | grep -v 'ext|spec|test' | \
xargs grep --files-with-match $'^\t' | xargs grep --files-without-match '^        ' | wc -l
66

Other 61 files do not indent. We should not bother them.

% git ls-files | grep '\.\(c\|h\|def\)$' | grep -v 'ext|spec|test' | \
xargs grep --files-without-match $'^\t' | xargs grep --files-without-match '^        ' | wc -l
61

So far so good. But what about remaining 101 files? The answer is obvious; these files MIX indents.

% git ls-files | grep '\.\(c\|h\|def\)$' | grep -v 'ext|spec|test' | \
xargs grep --files-with-match $'^\t' | xargs grep --files-with-match '^        ' | wc -l
101

This is totally wrong. No matter should we use spaces or tabs for indentations, it must be consistent.

History

#1 [ruby-core:84500] Updated by duerst (Martin Dürst) 28 days ago

I'm all for better consistency. I thought there were clear guidelines for Ruby, and (almost) everybody was following them. I didn't know it was that bad.

But fixing spacing will obscure where the code originally came from. I don't really like that.

To improve the situation, I have the following suggestions:

1) Install an SVN bot that fixes indents on newly committed lines automatically. That would hopefully catch the attention of the committers.

2) Based on svn blame, get a picture of who is how good (or not) at following the guidelines, and gently nudge everybody in the right direction.

[Somebody sooner or later will start discussion on what the best way of indenting should be. If you want to do this, please make it a separate issue. Please tag it as "joke" (because we don't have a "bikeshed" category).]

#2 Updated by shyouhei (Shyouhei Urabe) 28 days ago

  • Description updated (diff)

#3 [ruby-core:84506] Updated by normalperson (Eric Wong) 28 days ago

duerst@it.aoyama.ac.jp wrote:

I'm all for better consistency. I thought there were clear
guidelines for Ruby, and (almost) everybody was following
them. I didn't know it was that bad.

Right, I remember it being 4 spaces per-level, and use TAB
whenever it's 8 spaces. AFAIK, that's the Emacs default and I've
seen this in several other projects, so not uncommon..
In vim, I use: :set ts=8 sw=4 sts=4 noexpandtab

But fixing spacing will obscure where the code originally came
from. I don't really like that.

Agreed 100%. Noise makes code archaelogy harder, I don't like
whitespace-only changes; but I'd be happy to see them if one is
in the affected area and already changing code, and the proposed
SVN bot might help with that...

I admit that I moved some existing all-space code not long ago
and did not notice it, but nobu fixed it for me :x

To improve the situation, I have the following suggestions:

1) Install an SVN bot that fixes indents on newly committed
lines automatically. That would hopefully catch the attention
of the committers.

Instead of auto-fixing, it should email the committer;
that ought to reduce future screwups.

#4 [ruby-core:84561] Updated by znz (Kazuhiro NISHIYAMA) 25 days ago

I think that guidelines are .editorconfig for most editors, misc/ruby-style.el for Emacs, and .indent.pro for indent.

#5 [ruby-core:84653] Updated by vo.x (Vit Ondruch) 19 days ago

Let me quote Developers How-To 1:

  • indent
    • 4 for C
    • 2 for Ruby
  • tab/space
    • Do not use TABs in ruby codes ruby-dev:19388
    • Use TAB instead of 8 SPs in C. (Emacs's default style)

#6 [ruby-core:84658] Updated by naruse (Yui NARUSE) 19 days ago

Just FYI, there's a format tool named clang-format.
https://clang.llvm.org/docs/ClangFormat.html

I use following options:
https://github.com/nurse/strptime/blob/master/.clang-format

#7 [ruby-core:84857] Updated by graywolf (Gray Wolf) 10 days ago

As vo.x (Vit Ondruch) already pointed out, there are guidelines for the indentation. So based on the guidelines the 101 files are (probably) correctly indented and the 76 (10 + 66) files are (probably) wrongly indented.

#8 [ruby-core:84863] Updated by shyouhei (Shyouhei Urabe) 9 days ago

graywolf (Gray Wolf) we are talking about C codes here. The "Do not use TABs" policy applies to non-C files.

#9 [ruby-core:84864] Updated by graywolf (Gray Wolf) 9 days ago

I can't count to 8 in

xargs grep --files-with-match '^        '`

, sorry, just ignore me. :/

#10 [ruby-core:85033] Updated by shyouhei (Shyouhei Urabe) about 1 hour ago

We discussed this issue in today's developer meeting.

  • We agreed not to batch update the indents at once. Indents should become consistent occasionally.
  • Matz has no strong opinion on this topic.
  • We agreed to move to spaces only. Reasons behind this:
    • No contemporary advantage of mixing tabs and spaces today.
    • Emacs fans at the meeting could use ruby-style.el, and that style can be updated.

So the next action is:

  • Modify ruby-style.el and other config files.
  • Use them.

Also available in: Atom PDF