Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tabs vs spaces? #1709

Closed
Bartvds opened this issue Feb 18, 2014 · 45 comments
Closed

Tabs vs spaces? #1709

Bartvds opened this issue Feb 18, 2014 · 45 comments

Comments

@Bartvds
Copy link
Collaborator

Bartvds commented Feb 18, 2014

If was running some code against the definitions and was shocked how the repo is infested with mixed use of tabs and spaces.

We could organise a battle to decide on tabs vs space, but nobody wins there.

Maybe we should just stick with whatever style the original was created and enforce that?

I think we could combine detect-indent with TSLint in the tester (when we have #1314).

@luisrudge
Copy link
Contributor

There's no battle. Use tabs.

@johnnyreilly
Copy link
Member

I vote spaces!

Let the internicine conflict begin! 😄

@luisrudge
Copy link
Contributor

haha. we don't need that. But you know... we wouldn't be having this conversation if we were using tabs and every dev would still have his/her code perfectly aligned the way he/she wanted (2 spaces or 4 spaces). Just saying.

@vvakame
Copy link
Member

vvakame commented Feb 19, 2014

I like tab.
but I think that we should adhere to the default settings for VisualStudio.

@omidkrad
Copy link
Contributor

Spaces with indent size of 4 (VS defaults).
Set your IDE to do virtual tabs for you and everybody's happy.
I just don't like to see [tab] [tab] [space] [space] [space] for code that wants to align with a word in line above.

@Bartvds
Copy link
Collaborator Author

Bartvds commented Jul 3, 2014

If anybody feels like writing a TSLint rule to enforce a consistent style (eg: either tabs or 4 spaces but not both) then we'd run it in new tester (it has TSLint support).

See also palantir/tslint#122

@basarat
Copy link
Member

basarat commented Jul 3, 2014

Spaces! FWIW JS libs use spaces (angular / jquery)

This probably came about because of Crockford : http://javascript.crockford.com/code.html

The unit of indentation is four spaces. Use of tabs should be avoided because (as of this writing in the 21st Century) there still is not a standard for the placement of tabstops. The use of spaces can produce a larger filesize, but the size is not significant over local networks, and the difference is eliminated by minification.

Since TS is JS, we really should conform.

@Bartvds
Copy link
Collaborator Author

Bartvds commented Jul 3, 2014

Well, Crockford is a bit of a grumpy old guy so I take him with a grain of salt.

There was a statistic on Reddit a while ago where someone analysed a decent chunk of Github and it was like 60% / 40% in favour of spaces. Myself and all the places I worked so far were tabs (although big part of that was AS3). It works totally fine. Code alignment is a waste of time but if needed smart-tabs work fine there too.

The real bane I want to get rid of is mixed indents. Before we go thermonuclear on one or the other over the whole repo, at least it must be consistent per file.

At some point we'll do #2228 but that's quite a big step (screws a lot of attributions so we can let @dt-bot do it).

@basarat
Copy link
Member

basarat commented Jul 3, 2014

Well, Crockford is a bit of a grumpy old guy so I take him with a grain of salt.

agreed

@johnnyreilly
Copy link
Member

If you're ever bored I advise reading the pull requests made to JSON.js. invariably they are submitted by people who don't know of Doug's slightly crotchety ways. They are all refused point blank :-)

@basarat
Copy link
Member

basarat commented Jul 7, 2014

Just saw this : https://github.com/iplabs/typescript/blob/languageService-v2/src/services/languageService.ts#L199-L213 the defaults are 4 spaces with tabs converted to spaces

    export class EditorOptions {
        public IndentSize: number = 4;
        public TabSize: number = 4;
        public NewLineCharacter: string = "\r\n";
        public ConvertTabsToSpaces: boolean = true;

        public static clone(objectToClone: EditorOptions): EditorOptions {
            var editorOptions = new EditorOptions();
            editorOptions.IndentSize = objectToClone.IndentSize;
            editorOptions.TabSize = objectToClone.TabSize;
            editorOptions.NewLineCharacter = objectToClone.NewLineCharacter;
            editorOptions.ConvertTabsToSpaces = objectToClone.ConvertTabsToSpaces;
            return editorOptions;
        }
    }

@johnnyreilly
Copy link
Member

How satisfying @basarat!

@Bartvds
Copy link
Collaborator Author

Bartvds commented Jul 7, 2014

The CRLF's are a bit stupid but whatever.

Maybe we should just go for it. I'll try running typescript-formatter on everything and see what happens.

@Bartvds
Copy link
Collaborator Author

Bartvds commented Jul 7, 2014

Dear lord, what a slaughterhouse.. 562 total .d.ts files, with 446 files changed..

https://github.com/Bartvds/DefinitelyTyped/tree/34907936bfc22562f30bb09ee82282ca0515b8b3

Bartvds@3490793

Sorry, we could not display the entire diff because too many files (446) changed

Heh.

@Bartvds
Copy link
Collaborator Author

Bartvds commented Jul 7, 2014

And did the tests too:

https://github.com/Bartvds/DefinitelyTyped/tree/46f9d17b2b074c887bff35b06112070b28924248

Bartvds@46f9d17

Sorry, we could not display the entire diff because too many files (442) changed.

Nice. So we have the technology, that was the easy part. But now need to decide if this is something we want because it will have some serious impact.

For one line attribution is going to hell. And if we accept that then we should probably enforce formatting in the tester. I have TSLint setup in the tester (but it's disabled) so that can do a lot.

Maybe we should improve the tester to a CLI util that can run the formatter and execute the tests, in a nicely usable way (it sorta works now locally but should be improved with clean CLI api etc).

@omidkrad
Copy link
Contributor

omidkrad commented Jul 7, 2014

I say the sooner the better. But good news is that you won't loose the blame history. I wished GitHub had implemented this feature too in their diff view.

@Bartvds
Copy link
Collaborator Author

Bartvds commented Jul 7, 2014

Yea, but it sucks that git blame -w -M doesn't work online. I just emailed Github support about it.

@omidkrad
Copy link
Contributor

@Bartvds, any response from GitHub?!

@Bartvds
Copy link
Collaborator Author

Bartvds commented Jul 10, 2014

Ignoring whitespace on the GitHub blame view is not possible currently, but we might add it in the future. Thanks for the suggestion, I'll add it to the Feature Request List so the team can see it.

So might take a while, if ever.

@jednano
Copy link
Contributor

jednano commented Oct 11, 2014

I don't care what any of the supposed know-it-alls or style guides say – tabs for indentation are the only way to preserve the intent of the developer when alignment is involved. See my blog post about it.

use-tabs-for-indentation-spaces-for-alignment-anim

@basarat
Copy link
Member

basarat commented Oct 11, 2014

the code alignment (align on var and = in your case) concept is good. But a difficult one to maintain in refactorings. So I just don't use it. Also auto format tools don't respect it e.g. auto format code in TypeScript

Also for

  public string FirstName { get; set; }        =>  public  string  FirstName { get; set; }    
  public string Surname { get; set; }          =>  public  string  Surname   { get; set; }
  public int Age { get; private set; }         =>  public  int     Age       { get; private set; }
  private Address Address { get;  set; }       =>  private Address Address   { get; set; } 

Add a new property and you need to reformat all these lines. Not a good code change to review.

Your workflow might be different and might be allowing for this. It does make it much easier to read (code as art) but I haven't managed to experience teams moving tp it.

@johnnyreilly
Copy link
Member

It's the oddest thing. My instinctive reaction to idea of mixing tabs and spaces is to shiver and think "that's just filthy!"

Completely irrational I know - but entirely sincere. Humans are weird...

@jednano
Copy link
Contributor

jednano commented Oct 11, 2014

@johnnyreilly you are victim to a common misconception. Using tabs for indentation and spaces for alignment is NOT the same thing as mixing tabs with spaces. You clearly didn't read the blog post.

Using tabs for alignment is why so many have been turned off from tabs at all, because people were doing it wrong. Had they been doing it properly in the first place, nobody would have noticed when they changed their editor's preference to display tab width at a different size. This is what the animated gif, above, is trying to demonstrate, how people have been doing it wrong.

@jednano
Copy link
Contributor

jednano commented Oct 11, 2014

@basarat, I totally agree with you. I rarely work on projects with code alignment, but I do see it from time to time. If you could somehow enforce that developers on your project do not use alignment at all, then I'd say go for it. Otherwise, with spaces for indentation, there is no way for linting tools, among others, to distinguish between what was intended for indentation vs. what was intended for alignment, outside of parsing the code into CST (way overkill).

Of course, there are host of other reasons not to use spaces for indentation in this modern age. Editors deal well with tabs, but they never have with spaces. Arrow-key navigation, delete, backspace – they just don't treat one indent of spaces like one unit. I've used Visual Studio, Sublime, PHPStorm and Notepad++, but they all have this fundamental problem with spaces for indentation.

@basarat
Copy link
Member

basarat commented Oct 11, 2014

Arrow-key navigation, delete, backspace – they just don't treat one indent of spaces like one unit.

Navigation: I use ctrl + arrow keys. They hop spaces (all whitespace really) + words.

For delete / backspace: For word selection I use expand word (ctrl+w from resharper defaults) and unexpand selection (ctrl+shift+w). I rarely need to select inside whitespace though, just use tab (increase indent) and shift tab (decrease indent).

@johnnyreilly
Copy link
Member

@jedmao actually I understood your point - I was just sharing my instinctive (and irrational) reaction. I didn't say it made sense 😄

@jednano
Copy link
Contributor

jednano commented Oct 11, 2014

@johnnyreilly gotcha.

@basarat I use the Home and End keys in some of your whitespace scenarios. I have tried to use Ctrl + arrow key navigation but it doesn't always work. This is why I rarely use it out of mental consistency. Consider the following example:

for (var i = 0; i < 10; i++) {
    if (i > 5) {
        console.log('I am greater than 5');
    }
}

Say my cursor is at the beginning of line 3 and I want to navigate up to the beginning of line 2.

Using your technique in Sublime, I could do it one of 2 ways:

  • Up, Hold Ctrl, Left, Release Ctrl (4 logical steps)
  • Hold Ctrl, Left, Release Ctrl, Up, Hold Ctrl, Right, Left, Release Ctrl (8 logical steps)

In Visual Studio, the 2nd scenario can be reduced to 7 logical steps.

  • Hold Ctrl, Left, Release Ctrl, Up, Hold Ctrl, Right, Release Ctrl (7 logical steps)

With actual tab characters, the shortest path is only 2 logical steps with no modifier keys. Couldn't be simpler.

  • Left, Up

I fail to see how word selection would aid in deleting or backspacing a spaced indent. I'm trying it in Visual Studio with no success. This is why I created the TabSanity Plugin for Visual Studio. Unfortunately, I'm not aware of such plugins for other editors.

@basarat
Copy link
Member

basarat commented Oct 11, 2014

With actual tab characters, it's always 2 logical steps with no modifier keys. Couldn't be simpler

👍 I see your advantage. However as I edit this comment, ctrl etc works same as my VS so my habits carry over ;)

I fail to see how word selection would aid in deleting or backspacing a spaced indent

My bad, my description was confusing. Ignore the "word" selection bit. Just focus on I rarely need to select inside whitespace though, just use tab (increase indent) and shift tab (decrease indent). It works with spaced indents just fine.

@basarat
Copy link
Member

basarat commented Oct 11, 2014

With actual tab characters, it's always 2 logical steps with no modifier keys.

What about :

public  string  FirstName { get; set; }    
public  string  Surname   { get; set; }
public  int     Age       { get; private set; }
private Address Address   { get; set; } 

Wouldn't going from the beginning of Age to changing the type of Surname i.e. the string on the previous line be same as the Ctrl scenario?

@jednano
Copy link
Contributor

jednano commented Oct 11, 2014

@basarat I use Tab and Shift+Tab religiously; though, usually for multiple lines of code. I guess you could say that Shift+Tab is a good alias for a backspace keypress on a tab, but it's clearly not as simple or intuitive as a backspace.

Ctrl+Delete would be the closest alias to the Delete key, but it has the potential to delete more than one indent; whereas, a simple Delete key on a tab removes only one indent. Working sometimes and not others is why I don't use it at all (mental consistency).

I, like you, also rarely, if ever, need to select inside whitespace. I never suggested that I would. I'm not selecting the whitespace, but navigating through it to get somewhere. Your alignment example above is definitely when I would use Ctrl + arrow key navigation. This is why, as I said before, I rarely use Ctrl + arrow key navigation because the scenario you pointed out is probably the only case in which I would use it.

At the end of the day, what bothers me the most, is that you have to totally change your muscle memory when switching between a file with tabs for indentation vs. spaces for indentation. What's efficient (fewest logical steps) in a file with tabs for indentation simply does not work in a file with spaces for indentation. You could blame the editors for not giving you a seamless experience, but that's not fair because it's literally impossible (without CST) for editors to make assumptions in a file with spaces for indentation about what is an indent and what is alignment.

What's even more frustrating is that it's so hard for people to recognize it. Spaces for indentation are pure evil... with horns.

@BirdTho
Copy link

BirdTho commented Apr 29, 2015

Final conclusion? I just want to contribute, what the heck are you using?

@basarat
Copy link
Member

basarat commented Apr 29, 2015

Final conclusion? I just want to contribute, what the heck are you using

Consistent per file. Don't mix these in a file

@jednano
Copy link
Contributor

jednano commented Apr 29, 2015

@basarat the comic is wrong. We all know the Apple guy would use spaces and the Microsoft guy would use tabs.

@basarat
Copy link
Member

basarat commented Apr 29, 2015

@jedmao I'm an ms guy and I use spaces :)

@vvakame
Copy link
Member

vvakame commented Apr 29, 2015

I think #4211 is nice proposal.

@davidreher
Copy link
Contributor

I would propose to use 2 spaces as done in the airbnb/javascript styleguide ...

@glen-84
Copy link
Contributor

glen-84 commented Jan 1, 2016

Suggestion: Use the coding standards set out by the library being typed. If the library being typed uses spaces, single quotes, and LFs, then do the same in the type definition. This way we can avoid the spaces vs tabs argument.

The only problem is that it won't be easy to enforce this rule. If you need to enforce it and have full consistency, then I would recommend going with the most popular conventions. According to this website, it's spaces and single quotes.

My personal preference is spaces (4) and double quotes, but I'm okay with using single quotes if it means that everything is consistent.

Could the pro-tab and pro-tab-space users accept a similar compromise?

@basarat
Copy link
Member

basarat commented Jan 2, 2016

My personal preference is spaces (4) and double quotes, but I'm okay with using single quotes if it means that everything is consistent.

@glen-84 purely for your amusement ... here why the compiler team used double quotes : microsoft/TypeScript#5811 (comment)

  • JSON
  • Works across all languages so less of a cognitive load

I personally go with single quotes myself as I find myself doing more and more TypeScript exclusively 🌹

@glen-84
Copy link
Contributor

glen-84 commented Jan 2, 2016

I think it's also a bit more legible, and it was 57% vs 43% (not a big difference), but I guess you have to draw the line somewhere. =)

@GeertJohan
Copy link

I like @jedmao's point on tabs for indentation and spaces for alignment.

Full disclosure: I'm coming from the Go-land where we have go fmt and no discussions about tabs or spaces ever.. It's tabs for indentation and spaces for alignment and it works very very well 👍

@hinell
Copy link
Contributor

hinell commented Feb 7, 2017

Spaces of course. Cause nobody knows how the tabs are exactly rendered in different editors if they are used elsewhere.

@jednano
Copy link
Contributor

jednano commented Feb 7, 2017

@hinell why does it matter if tabs are rendered differently in different editors? It shouldn't, so long as tabs are "only" used for indentation.

@ExE-Boss
Copy link
Contributor

ExE-Boss commented Mar 8, 2019

I prefer tabs, especially because of this: http://nickgravgaard.com/elastic-tabstops/

@ExE-Boss
Copy link
Contributor

ExE-Boss commented Feb 8, 2020

Turns out, there’s an even bigger reason to use tabs: developer accessibility.

@jakebailey
Copy link
Member

The repo was formatted consistently using dprint a few months ago, so this is settled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests