tldr-pages / tldr-c-client

C command-line client for tldr pages 📚
MIT License
304 stars 50 forks source link

Using tabs for vertical alignment is fragile #3

Closed waldyrious closed 8 years ago

waldyrious commented 8 years ago

(see e.g. how the current version of this file displays on github)

Leandros commented 8 years ago

It might seem fragile, but it's required for make to work.

waldyrious commented 8 years ago

Oh, interesting, I didn't know that. In that case, what tab size are you using locally? It shows up misaligned on the github interface, any idea how that could be mitigated? I wonder if Github respects an .editorconfig file...

Btw, are the tabs before the equal signs also required by make, or just the ones after each target? (sorry, make noob here.)

igorshubovych commented 8 years ago

I also learned about it recently.

This what I had to add to .editorconfig for tldr repo.

[Makefile]
indent_style = tab
indent_size = 8
Leandros commented 8 years ago

@waldyrious IIRC, make only requires you to use tab after a target, for example, here:

install: all
    install -d $(BINDIR)
    install tldr $(BINDIR)

@igorshubovych

That's interesting, I didn't knew about .editorconfig files. I solely rely on my vim config.

igorshubovych commented 8 years ago

Check it out: http://editorconfig.org/

And there is vim integration too https://github.com/editorconfig/editorconfig-vim#readme

waldyrious commented 8 years ago

@igorshubovych so such a file added to this repo would make github display the tabs correctly?

In any case, I still think the tabs in the variable definitions should be replaced by spaces, since that's vertical alignment, not indentation. But I don't want to start a flame war here if @Leandros' preference is to always use tabs :)

Leandros commented 8 years ago

Well, tabs vs spaces is a long running war, and I really do not wanna have one in small right now. ;) I see your point, it's ugly and looks broken in github, but tabs are required in makefiles (and every editor defaults to tabs in makefiles), and breaking it up, to allow spaces in the assignments, but use tabs in the targets is just ridiculous.

waldyrious commented 8 years ago

Fair enough. Just a friendly note, though: as a contributor to many projects with small patches like this one, I've developed a bit of thick skin over the years, but others (as I did in the beginning) may become strongly discouraged by getting their contribution or good-faith suggestions called "ridiculous". Btw, I mean no sarcasm or passive agressiveness here (I know it's tough to communicate online by writing), just a heads up to look out for that sort of language, especially regarding volunteer contributions.

Getting back to the issue at hand, if @igorshubovych confirms that his suggestion would work for github to present the file on the github interface without the need to change it, would you be willing to consider such a PR (adding an .editorconfig file)?

igorshubovych commented 8 years ago

No, I doubt Github has any integration with EditorConfig. I added it for myself, so I will not break Makefile.

waldyrious commented 8 years ago

Ok, I suspected as much. Thanks for confirming.

Leandros commented 8 years ago

I wasn't calling your suggestion ridiculous, @waldyrious, but to have different indentation rules in one file, based on context.

waldyrious commented 8 years ago

I understand that (thanks for the assurance nevertheless), but the word is a bit strong, and could touch a nerve if read the wrong way, unnecessarily. In any case, I share your aversion for context-sensitive markup, and since the presentation can't be adjusted (yet! I wouldn't be surprised if github eventually starts supporting .editorconfig), we can close this discussion. Cheers!