Handling inconsistently indented files?

[tpope]

Handling inconsistently indented files?

[karptonite]

That makes sense.
I don't want to argue the point too much, since you've obviously given it a lot of thought, but if it make sense to set tabstop to 8 spaces when tabs and spaces are mixed, why does it make sense to use the user provided tabstop setting when tabs are used exclusively? Why are the situations treated differently?

[scottchiefbaker]

I'm having the same issue as @karptonite. I have some code that has a mix of tabs and spaces. I'd really like an option in vim-sleuth that lets ME pick what the settings should be in that case.

Right now it sets tabstop to 8 which is definitely NOT correct for my code. In fact that's how I know the code has mixed tabs/spaces because my indenting goes WAY to the right.

[tpope]

Historically tabs have been used as a stand in for 8 spaces. The Vim source code itself is formatted this way. Lots of projects are formatted this way.

The heuristics could potentially be improved by counting lines with 8 or more spaces, but I'm not sure what the specifics would look like.

Closing as not actionable but feel free to discuss further.

[tpope]

I think overriding 'tabstop' is a bad practice in light of tab's historical baggage, but if the user has chosen to do so, I've reluctantly agreed not to override it in cases where a file is 100% tabs, since it doesn't really hurt anything.

[krader1961]

FWIW, I agree with Tim. Mixed tabs and spaces are problematic and overriding tabstop from its historical value of eight is a really bad idea. And I say that as someone who in the distant past evangelized for the use of tabstop=4 and used that in my C coding. Then I started contributing to open source projects like the Linux Kernel and learned the errors of my way. Do yourself a favor and, more importantly, anyone else who might ever look at your code and use only leading tabs or spaces but not both. The space savings afforded by tabs was justified 20+ years ago but not today and I'm not aware of any mainstream languages that require their use. I vote that this issue be closed.

[tpope]

Meant to close as mentioned above but forgot to hit the button. But yeah, open to further concrete suggestions, but I will never support heuristics or even an option that breaks consistent files in order to coddle inconsistent ones, because people will set it without understanding the consequences. Possible workarounds:

1. Fix the damn files.
2. Use modelines.
3. au BufReadPost /glob/matching/broken/files set tabstop=7 shiftwidth=5 noexpandtab

[scottchiefbaker]

I contribute to a lot of projects all with varying indent styles. I'd like vim-sleuth to:

1. Check if the indenting is tabs
   1. noexpandtab
2. Check if the indenting is some number of spaces
   1. expandtab
3. It's a mixture of both #1 and #2
   1. Don't do anything, use the defaults I have set in Vim already.

For #3, what I get now is shiftwidth 8, which is NEVER right for the code I work on. If #3 occurs I'd have no problem if it popped up and said "inconsistent formatting not changing indent settings". That would give me the clue to go and FIX the code.

What happens instead is I get set to shiftwidth 8 and when I try and fix the code using equals for formatting is messes the code up even worse.

[tpope]

See my first comment.

[tpope]

To make myself crystal clear, I'm not going to demote the large set of projects using tabs as 8 spaces to second class citizens because you NEVER happen work on them. First comment explains I'm willing to entertain a solution that only applies to projects with a line with more than 8 spaces, since that shouldn't happen in the tab as 8 spaces case.

[krader1961]

@scottchiefbaker You're arguing for behavior that is at odds with long established practice. It's also an argument akin to whether vim, emacs, or MS-Windows notepad is the better editor. If what vim-sleuth does is never right for the code you work on might I respectfully suggest you use something else? Perhaps fork the project and modify it to suit your needs?

[tpope]

Or actually engage on the point I'm making rather than demanding an option. This is solvable.

Here's the logic.

• hard is number of lines with leading tabs.
• soft is the number of lines with 8 or more leading spaces (and thus the most relevant to this discussion).
• spaces is the number of lines with 2 or more leading spaces.

[karptonite]

I don't want to suggest making changes that will make Vim Sleuth less useful for anyone. However, I am confused about the current behavior.

Say I have tabstop, shiftwidth, and softtabstop all set to 4, and I open a file that uses mostly tabs, for spacing, but has a couple lines that start with spaces. At that point, Vim sleuth will leave shiftwidth and softtabstop as 4, and change tabstop to 8.

What is the use case where it makes sense to change tabstop, but NOT shiftwidth? Are there projects that use 4 space indents normally, but also mix in tabs assuming they will be 8 spaces? (Not a rhetorical question--I've never seen that mix.)

Incidentally, contrary to what @scottchiefbaker wrote, I don't see any problem with the handling of shiftwidth. It is only tabstop that causes problems for my workflow. I'd love to find a solution that retains the default tabstop of 8, while allowing users who understand why they are doing so to override it.

(And as for @krader1961 's suggestion of using tabs and spaces consistently in our code, I do, in my own code. But I have to work with other people's code, and I'd like to be able to open a file to work on it without having to shift contexts to thinking about tabs and spaces when I want to be thinking about code.)

(@tpope, it is probably obvious, but I wrote my post before seeing yours.)

[tpope]

It must be changing it to 4, not leaving it at 4. There is no scenario I can find where it will set tabstop without setting shiftwidth. For this specific problem, you could probably tweak the if heuristics.hard line. Maybe something like

if heuristics.hard && (!heuristics.soft || get(heuristics, 'shiftwidth') != &tabstop)

"If we are strictly hard tabs, or partially hard tabs and the shiftwidth differs from the tabstop, change the tabstop to 8."

[tpope]

Another approach might be to add a bit of fuzz to the first conditional.

if heuristics.hard > 19 * heuristics.spaces

"If we're 95% hard tabs, set the shiftwidth to tabstop."

I worry a bit about false positives here so I want to keep that percent as high as possible.

[karptonite]

Yeah, it looks like you are right--my mistake. It was "changing" it from 4 to 4, because the shiftwidth heuristic got it right. I just assumed it was leaving it.

Both solutions you propose look like improvements to me, although I'd have to see them in practice. Each would help with different cases: The first will definitely work for cases where the heuristic correctly predicts the softtabstop. And the second will work for cases where just a couple of lines with spaces slip through (and thus the softtabstop heuristic might otherwise be wrong).

I'd argue for combining them, I think.
if (heuristics.hard > 19 * heuristics.spaces) && (!heuristics.soft || get(heuristics, 'shiftwidth') != &tabstop)
Not the prettiest condition, but it might make everyone happy.

[tpope]

I intended that fuzz for the outermost conditional, although perhaps there's a reason to move it inside.

[karptonite]

Hmm... not sure. Might be good to leave it outside. I realize now that they are intended to have different consequences. One to set the tabstop, one to set softtabstop. Yeah--looking again, that looks right to me, having it outside.

[karptonite]

@tpope I gave your suggestions a try here:
https://github.com/karptonite/vim-sleuth
Seems to work great for me! I added shiftwidth to the heuristics.

[tpope]

Please try each separately for a few days and report back. I'll do the same.

You can't add shiftwidth there; it will never get overriden.

[karptonite]

Thanks, you're right. Would this comparison be OK, then?
if heuristics.hard && (!heuristics.soft || get(options, 'shiftwidth') != &tabstop)
and not adding shiftwidth to he heuristics at all? It should be set by that point.

Regarding trying each separately, I already tried just the 95% fuzz on the first conditional--it is not sufficient for cases where there is a substantial mix of spaces and tabs. I'll try just the inner tweak instead.

Edit: I've removed the outer fuzz from my branch, and changed the comparison to shiftwidth directly in the options. I'll work with that for a while, if you don't see any problems with it.

[tpope]

Shiftwidth has already been added at that point, except in the edge case of no indenting in the entire file (at which point sleuth will check another file).

[karptonite]

So I've been testing for a couple days with just the inner comparison. It worked well for some files, but today I ran into one that it didn't work for. The stats for that file are:

heuristics.hard: 268
heuristics.soft: 0
heuristics.spaces: 36

options.shiftwidth at that point is 3, the correct value. (I've been saying 4 for convenience, but in fact our company uses 3 in our files).

The outer conditional evaluates to false because heuristics.spaces is too large (and would be too large even with the 95% fuzz you suggested).
heuristics.soft is also zero, because no one line was indented more than 8 spaces.

Would you be open to changing that inner conditional to this?
if heuristics.hard && (!heuristics.spaces || get(options, 'shiftwidth') != &tabstop)
That would work for my file, but I'm not sure if that would lead to other problems.

[karptonite]

Totally separately, what is this conditional for?

vim-sleuth/plugin/sleuth.vim

Line 81 in 6b0c9d6

elseif heuristics.soft != heuristics.hard

You are comparing heuristics.hard to heuristics.soft for equality; are you just looking for empty files?

[tpope]

That replacement conditional would match on every single file using tabs as 8 spaces and a non-8 shiftwidth, no bueno. For cases where soft is zero, I don't think this is solvable under the constraints I have placed.

The linked conditional most usefully matches empty (or unindented) files. More generally, any time there's an exact tie, punt so another file can be checked.

[karptonite]

Makes sense.

It isn't totally unsolvable--that 95% fuzz won't work for the file I had, but would have worked if there had been fewer (but not zero) leading spaces, or if the fuzz were more like 85%. But at that point, maybe you'd start getting those false positives you were worried about. And of course, it is a bit arbitrary to base this on one bizarrely spaced file.

Anyway, the inner change is working in many cases, I think.

[karptonite]

Today I ran into a file that had heuristics.soft of zero, heuristics.spaces of 1, and heuristics.hard of about 600. Sleuth overrode the tabstop to 8, but when I added the outer 95% fuzz, that solved the problem. It looks like both changes you suggested help with different files. I now have both in my branch, if you want to try them.

[karptonite]

Any thoughts about whether this is a change you are willing to add to vim-sleuth permanently? While the two changes still don't handle all files correctly, it is a substantial improvement for me.

[tpope]

I think some variant will eventually make it in. I have a couple more ideas I want to think through:

1. If the file is mixed, check another file, but save our best guess lest every file be broken. (This is indirectly inspired by and perhaps complementary to #13).
2. If the buffer local 'tabstop' or other relevant option has been changed (presumably from a FileType event), that could trigger more aggressive overriding. The idea is that if you change the global 'tabstop', you aren't taking into account the historical baggage, but if you change it in, say, JavaScript files only, you're targeting a more specific problem. I guess my first question here would be what file types are giving people issues.