Misc #18891: Expand tabs in C code

• Status: Closed
• Assignee: -

[k0kubun (Takashi Kokubun)]

Problem

There's no way to handle tab/space-mixed indentation of CRuby's C code properly in VSCode. It impacts the productivity of the VSCode users a lot, including myself.

Suggested solutions

I implemented option 2 out of the following options at https://github.com/ruby/ruby/pull/6094.

1. You tell me how to do that in VSCode.
   • In VSCode, there seems to be no native feature to show tabs as 8 spaces while still indenting with 4 spaces. At least https://github.com/microsoft/vscode/issues/42740 is still open.
   • @alanwu (Alan Wu) built https://github.com/XrXr/vscode-tabs-are-two-indents exactly for this purpose, but even with the extension, it still seems to wrongly detect an indentation level on new lines.
2. Expand all tabs in most of the C code at once and add the commit to .git-blame-ignore-revs
   • pros: GitHub and git blame --ignore-revs-file support it. Unlike option 3, commit logs and .git-blame-ignore-revs can be short.
   • cons: It might be a bit difficult to find out which C files shouldn't be a target. Bare git blame could be inconvenient.
3. Allow expanding all tabs in each C file as we want and manage the list of such commits in .git-blame-ignore-revs
   • pros: GitHub and git blame --ignore-revs-file support it. You don't introduce unnecessary changes to files you don't care about.
   • cons: We might see too many commits for this in the commit history. Bare git blame could be inconvenient for such files.
4. Discourage contributions to CRuby from VSCode users. (proposed by @graywolf (Gray Wolf))

Drawbacks

The expected drawbacks of doing option 2 are:

• Existing patches on hard-tab lines may conflict.
• Backporting patches on hard-tab lines made after this commit to revisions before this commit may conflict.

I assume that resolving such conflicts wouldn't be too difficult and this problem will not last forever.

Out of scope

I'm not interested in discussing the adoption of clang-format. It would change a lot of things that are not related to the problem.

Past discussion

https://bugs.ruby-lang.org/issues/16112 touched this topic before, but we haven't discussed --ignore-revs-file so much. I also didn't know VSCode still can't deal with it in 2022 (as I've been using Vim).

[graywolf (Gray Wolf)]

One more solution would be:

4. Do not use broken editors.

¯\_(ツ)_/¯

Should be at least listed here as an option.

[k0kubun (Takashi Kokubun)]

• Description updated (diff)

Sure, I added your solution to the description.

[Eregon (Benoit Daloze)]

Thank you for the clear and precise issue.

(2) seems very obviously the way, and it's amazing it has not been done yet.
This is the #1 problem for new (and some of the existing as well) contributors to CRuby..
If CRuby does not do this, it is explicitly rejecting new contributors for no good reasons.

(IMHO mixing spaces and tabs has no sense nowadays, it's an antique legacy non-sense practice with zero advantages, and there is no valid reason to keep that monster alive)

[k0kubun (Takashi Kokubun)]

It might be a bit difficult to find out which C files shouldn't be a target.

I noticed ruby-commit-hook/bin/auto-style.rb already knows about this. (2) doesn't have any blocker.

So I prepared a pull request: https://github.com/ruby/ruby/pull/6094. I confirmed GitHub ignores it just fine. If you have git config --global blame.ignoreRevsFile .git-blame-ignore-revs git config blame.ignoreRevsFile .git-blame-ignore-revs, git blame ignores it properly too. I'm inclined to move forward with this unless somebody has a problem with leaving such a commit.

[k0kubun (Takashi Kokubun)]

• Description updated (diff)

[k0kubun (Takashi Kokubun)]

• Description updated (diff)

[hsbt (Hiroshi SHIBATA)]

I'm +1 for https://github.com/ruby/ruby/pull/6094

[eightbitraptor (Matthew Valentine-House)]

I'm also in favour of https://github.com/ruby/ruby/pull/6094 - It feels like the most elegant solution of those proposed

[nagachika (Tomoyuki Chikanaga)]

I also support the 2nd solution from the perspective of stable branch maintenance.

[mame (Yusuke Endoh)]

@nagachika (Tomoyuki Chikanaga) Thank you for your comment. I vote for 2 too.

If CRuby does not do this, it is explicitly rejecting new contributors for no good reasons.

I want to use VSCode, but this expression is not correct in my perspective. VSCode is explicitly rejecting new users who have been working on any project with emacs-style indentation.

[mame (Yusuke Endoh)]

• Status changed from Open to Assigned

We discussed this issue at the dev meeting.

Finally, we were able to reach an agreement to expand hard tabs. Congratulations!

[etienne (Étienne Barrié)]

If you have git config --global blame.ignoreRevsFile .git-blame-ignore-revs, git blame ignores it properly too.

Unfortunately you can't set it at the global level because git blame fails if the file does not exist, and that would cause issues when working on other repos (see this discussion in the Git mailing list). But you can definitely add it as a config local to the repo, and mention how to do that at the top of the file itself.

I would also recommend setting these two configs which can help in case the ignored revisions end up hiding the origin of a line: blame.markUnblamableLines and blame.markIgnoredLines. Looking at the output of git blame at least you'll see that a revision was ignored. These two can be set at the global level.

[k0kubun (Takashi Kokubun)]

Unfortunately you can't set it at the global level because git blame fails if the file does not exist, and that would cause issues when working on other repos

Right, I noticed that behavior after writing the comment but forgot to update the comment about it. I corrected the comment to not use --global, which would write .git/config. Given that you could always use ~/.gitconfig or .git/config for your original git configuration, I guess it's fair to just commit .gitconfig to the repository. I'll add that once I merge it. (edit: I noticed committing .gitconfig doesn't work and you'd need to do something in .git/config like including it anyway) I'll leave a comment about it in .git-blame-ignore-revs. Hopefully, you wouldn't need to initialize your repository so often.

I would also recommend setting these two configs which can help in case the ignored revisions end up hiding the origin of a line: blame.markUnblamableLines and blame.markIgnoredLines.

Good to know. I guess the change of this time doesn't introduce new lines, but I'll consider adding that when I see something weird that can be solved by that.

[k0kubun (Takashi Kokubun)]

• Status changed from Assigned to Closed

Applied in changeset git|5b21e94bebed90180d8ff63dad03b8b948361089.

---

Expand tabs [ci skip]

[Misc #18891]

[k0kubun (Takashi Kokubun)]

I forgot to mention that I appreciate everybody who supported this decision. Thank you so much.