Financial-Times / origami-build-tools

Standard Origami component development tools.
47 stars 15 forks source link

Lint tabs vs. spaces so that we can have consistent indentation across Origami #55

Closed matthew-andrews closed 10 years ago

matthew-andrews commented 10 years ago

I've used this and it's quite nice. https://github.com/evanshortiss/lintspaces-cli

Suggested use of lintspaces (probably a better way to do files but good enough to test hack):-

lintspaces -n -t -d "tabs" -i js-comments `find . -name "*.js" ! -path "./node_modules/*" ! -path "./bower_components/*"`

I would have made a pull request but it's really quite tricky to add tasks. Raising a separate issue about this.

[Full disclaimer - even though I am suggesting tabs here, in my own time, I use spaces: https://github.com/matthew-andrews/offline-todo-api/blob/master/package.json#L9]

triblondon commented 10 years ago

Agreed, good idea.

matthew-andrews commented 10 years ago

Sadly I don't think this module is quite ready for the prime time - see https://github.com/schorfES/node-lintspaces/issues/7.

Would be curious to know if anyone knows of any other similar modules. I might just fork lintspaces - how hard could it be....

matthew-andrews commented 10 years ago

It can be done! I've updated the issue.

triblondon commented 10 years ago

Got to be a bit careful here since converting of existing code to Origami components could be hampered by linting rules like this. That said, the ads code doesn't currently pass linting anyway. I suggest we have a config option for it, and make it default-on.

matthew-andrews commented 10 years ago

Suggestion:-

AlbertoElias commented 10 years ago

@matthew-andrews Is #95 good enough to close this?

roland-vachter commented 10 years ago

Hi all,

I just found out that all module developers are forced to use tabs instead of spaces. I can't find any discussion in the ft-origami group, so I'll ask here. Can someone please explain the reason why tabs should be used instead of spaces? As a general technology best practice is to use spaces (so far that is what I knew).

Thanks

triblondon commented 10 years ago

Hi Roland,

The argument for a particular kind of indent is not clear cut at all, and I'd prefer it if it were, because then we wouldn't have this perennial and extremely boring discussion all the time.

I surveyed the FT and found we use a huge variety of indent styles including the team that uses 3 spaces because the two main devs could not agree on whether to use two or four.

For Origami we simply need to pick a setting and stick with it. We picked tabs, so that developers can use any tab width that they want.

This is really not a big deal, and you're not being forced to do anything. If you insist on using spaces, fine, just make sure you have your editor convert them to tabs on save.=

roland-vachter commented 10 years ago

Thanks for the answer Andrew. True, I was wrong saying best practice, it is more like a flame war :) I'll try to configure my editor so that it doesn't disturb me.

roland-vachter commented 10 years ago

Just as a heads-up, I also made a survey here in the office and among the web dev guys, I couldn't find anyone to use tabs. Everybody uses tabs converted to spaces (4 generally).

I found an issue with this, I guess this applies to JS files as well: in-line documentation for docs generators. image

The editor generates tabs a combination of tabs with different length and spaces, probably because it can't decide what should do. And I'm sure there are other issues with tabs ..

roland-vachter commented 10 years ago

It doesn't complain about spaces in JS files for indentation. Does that mean that we use tabs for SCSS files and personal preference for JS files?

matthew-andrews commented 10 years ago

These were the results from the tabs/spaces survey from the Next FT team. As you can see, the vote was so close we had to agree to disagree ;-).

img_6361

The behaviour you're describing sounds like a bug:- https://github.com/Financial-Times/origami-build-tools/blob/master/config/.editorconfig#L8

Looks like it should be tabs for .scss and .js files - but note it will allow spaces for jsdoc comments.

roland-vachter commented 10 years ago

Another example: image

Press a tab, then a space, then a tab again. It inserts a shorter tab, which could be interpreted differently in different environments. I also tried to copy/paste a code snippet in github for example, it removed all tabs and the result is an unindented and unreadable code.

Whay can't we make an FT dev wide survey? All web devs, Java devs, all programmers. I think that would be more relevant (Origami services are done by Java devs as well, but their opinion counts anyway).

matthew-andrews commented 10 years ago

Press a tab, then a space, then a tab again.

When would you do that? (Note, OBT already should exclude jsdoc comments from linting so that - as far as I know - you can do what you want within them)

It takes a little more setup for each editor - but tabs are the right default because people can choose how much indentation their editor displays them as to their own personal preferences, which is just not possible with spaces - which are not flexible or customisable at all.

FT wide dev survey doesn't feel like a great use of time… At the end of the day this sort of thing is really arbitrary, really doesn't matter and would invite a lot of bike-shedding. If a component author really, really strongly objects they can opt out (by creating their own .editorconfig file).

The problem at the moment is that it's chaos - a mixture of all sorts of indentation varying on a per file basis and my editor just goes nuts when I open any file. This impacts productivity and makes me want to add a link to another Wikipedia article on psychology.

This will fix that, provide a sensible default and a way to opt out - and if then 90% of component authors then override it to use spaces then we can (and should) consider changing the default but let's tidy up what we have right now first.

triblondon commented 10 years ago

There is no value to having this discussion, Roland. I appreciate your preference is different, but the majority view is actually not really that relevant. People move from one project to another with generally higher frequency than the coding standards of any one project change.

I appreciate your enthusiasm for the quality of our code, but the indent argument is NOT about whether to use tabs or spaces, it's entirely about everyone using the same thing, and it doesn't matter what that is. For Origami, that decision is now made. This is great news, because the thing that is important - consistency - is now settled.