Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

LF at the End of File #20

Closed
twirl opened this issue Dec 25, 2014 · 49 comments
Closed

LF at the End of File #20

twirl opened this issue Dec 25, 2014 · 49 comments

Comments

@twirl
Copy link

twirl commented Dec 25, 2014

Why using this rule in 2014?
Are you still making one large js file with cat?

@dmikis
Copy link
Contributor

dmikis commented Dec 25, 2014

First of all, a whole bunch of unix utils rely upon the fact that every line in a text file ends with newline. Second, some editors inserts last newline automatically, so you may accidentally become author of the last line.
This rule has the very same meaning as not BOM rule, UTF8 as an encoding and prohibition of trailing whitespaces: keep project files in a uniform and most portable way.

@twirl
Copy link
Author

twirl commented Dec 25, 2014

What are those UNIX tools?
If we say we need portability, then why LF not CR LF?

@dmikis
Copy link
Contributor

dmikis commented Dec 25, 2014

What are those UNIX tools?

Mainly, editors: they just add last newline on saving a file (and you'll become the author of the last line). According to POSIX in Unix-like systems text file is the one with newlines at the end of every line, including last line. Otherwise a program has every right to treat a file as a binary one.

If we say we need portability, then why LF not CR LF?

CR LF is Win-only, whereas LF is for everything else we actually use. Majority wins.

@twirl
Copy link
Author

twirl commented Dec 25, 2014

What's the problem with being an author of empty line? And why state this in a codestyle?

@dmikis
Copy link
Contributor

dmikis commented Dec 25, 2014

Not empty but the last one, which can be meaningful.

@twirl
Copy link
Author

twirl commented Dec 25, 2014

Meaningful to people? How?

@dmikis
Copy link
Contributor

dmikis commented Dec 25, 2014

You know you can write code and the last line of a file? :)

@twirl
Copy link
Author

twirl commented Dec 25, 2014

I don't understand.

@dmikis
Copy link
Contributor

dmikis commented Dec 25, 2014

I don't want to argue about this anymore. There is an implication from POSIX, which may or may not be strictly necessary, and since we mostly use POSIX-compliant tools I suggest this rule continues to live. There is no any reason whatsoever to change this, only nasty little repercussions.

@twirl
Copy link
Author

twirl commented Dec 26, 2014

We live without this rule, and everything is fine. Problem is greatly exaggerated, no contemporary tool gives any damn to absence of blank line.

@dmikis
Copy link
Contributor

dmikis commented Dec 26, 2014

Again, what about ones adding the last newline?

@twirl
Copy link
Author

twirl commented Dec 26, 2014

See no problem with them (== with being an author of blank line).

@dmikis
Copy link
Contributor

dmikis commented Dec 26, 2014

It's not just that: you may accidentally commit files you haven't actually changed.

@twirl
Copy link
Author

twirl commented Dec 26, 2014

Does this impose any problem?

@dmikis
Copy link
Contributor

dmikis commented Dec 26, 2014

I believe it does. SCMs are not only for ability to revert some changes, they're also able to provide sometimes crucial info about a piece of code. Thus checking in unrelated crap is not exactly desired.

@twirl
Copy link
Author

twirl commented Dec 26, 2014

git blame and svn blame provide information regarding every single line, so you wouldn't have problem with unrelated crap.

@dmikis
Copy link
Contributor

dmikis commented Dec 26, 2014

You do understand that would be informational noise which would not ease an investigation.

@twirl
Copy link
Author

twirl commented Dec 26, 2014

Requirement to add LFs to all files makes much more noise.

@dmikis
Copy link
Contributor

dmikis commented Dec 26, 2014

No, it's not. One in a lifetime setting to your editor.

@twirl
Copy link
Author

twirl commented Dec 26, 2014

Nope. As you mentioned, there are tons of different tools, some of them add LF, and some do not.

@dmikis
Copy link
Contributor

dmikis commented Dec 26, 2014

Actually, if you don't use windows, only tools we're using that don't add the last LF by default are WebStorm and SublimeText (these are known to me).

@tarmolov
Copy link
Member

I think LF is more convenient because:

  1. The majority of our developers use OSX or *nix. Not Windows.
  2. Our production code is deployed to servers with unix like systems.

Using LF is a good rule in my opinion.

@twirl
Copy link
Author

twirl commented Dec 29, 2014

Please, explain the logic to me then:

  1. Codestyle requires to have blank line at the end of the file because some (sic!) tools may require it, and you want to have as much portability as possible.
  2. Codestyle requires to use LFs instead of CRLFs, though some tools obviously won't work with LFs. There you somehow don't need portability.
    So, do you need portability or not?

@dmikis
Copy link
Contributor

dmikis commented Dec 29, 2014

Yeah, "portability" might be a little stretch. What I meant is portability across POSIX-compatible systems we use.

Never the less, I think I've exhausted my arguments here. If the majority of concerned people is pro removing the rule, let's just do that.

@twirl
Copy link
Author

twirl commented Dec 29, 2014

Let me note that other teams could have just opposite situation, i.e. much more windows than osx/linux systems.

@ghost
Copy link

ghost commented Dec 29, 2014

I vote for remove this rule from codestyle.

@tarmolov
Copy link
Member

my vote: keep the rule in the codestyle

@vtambourine
Copy link
Contributor

Keep the rule.

@dodev
Copy link
Contributor

dodev commented Dec 30, 2014

Windows IDEs and editors can be configured to use POSIX EOL characters and automatically place new line chars at the end of files in a few clicks. @twirl why is this an issue?

P.S. Even git will fire warnings if the file doesn't end with a new line.

@twirl
Copy link
Author

twirl commented Dec 30, 2014

Not every Windows tool could be configured to use LFs.

@dodev
Copy link
Contributor

dodev commented Dec 30, 2014

Notepad can't for sure.
What is your suggestion?

@twirl
Copy link
Author

twirl commented Dec 30, 2014

My suggestion is: do not state rules which are defined by developer tools stack since different teams use different stacks.

@dodev
Copy link
Contributor

dodev commented Dec 30, 2014

Yep, you're right, but I think EOL in the end of file is a good rule to follow.
We should decide if we should restrict the EOL character to POSIX, make POSIX EOL a recommendation or not state it at all.

@twirl
Copy link
Author

twirl commented Dec 30, 2014

We use POSIX systems and we haven't been facing any problems for at least last 3 years.

@dodev
Copy link
Contributor

dodev commented Dec 30, 2014

That doesn't guarantee that you won't in the future.
I think we should change the rule to It is recommended to end files with a line break character. Not specifying whether it should LFCR, CR or LF.

PS we should really discuss what goes in the codestyle - only how to write JS code, source file structure, Web API recommendation etc. I got really confused after all the issues you created.

@tarmolov
Copy link
Member

For me personally, it's a good practice to standardize an end of line:

  • Easier to work with multiline strings (you have only one char marks an end of line 😄 ).
  • Decrease possible commits diffs (developers work with different an end of line preferences can commit unnecessary changes).

I suggest to keep this rule for a while. It's not a big deal to support it.
if other teams are nervous about this rule too, we'll go back to this discussion.

What do you think?

@twirl
Copy link
Author

twirl commented Dec 30, 2014

I think the issue is about trailing blank line, not LF vs CRLF (that's just side story).

@dmikis
Copy link
Contributor

dmikis commented Jan 12, 2015

My final suggestion is to keep this rule if we OK with unix and cli specific aspect in the codestyle. If we do not want to state this, it's may be worth to remove the rule. Also it applies to no-BOM and LF-only rules.

@tarmolov
Copy link
Member

I agree with @dmikis. Keep it.

@twirl
Copy link
Author

twirl commented Jan 13, 2015

Okay. So, who makes final decision?

@alt-j
Copy link
Member

alt-j commented Jan 13, 2015

I think @ikokostya must, because he is primary author of this codestyle.

@twirl
Copy link
Author

twirl commented Jan 13, 2015

Hmm. Does @ikokostya act on behalf of all Yandex JavaScript developers?

@vtambourine
Copy link
Contributor

@dmikis, @tarmolov and me voted for keeping the rule. @dodev wasn't so explicit, but he said "I think EOL in the end of file is a good rule to follow", so I will count him on that side :)
@twirl and @zloylos voted for removing the rule.
@alt-j suspends.

So, majority of participants of that discussion supports that rule. This issue can be closed.

@tarmolov
Copy link
Member

Don't hurry, please ;)

@tarmolov tarmolov reopened this Jan 13, 2015
@qfox
Copy link

qfox commented Mar 9, 2015

@twirl I'm not sure why you against this rule.

Pros:

  • POSIX tools that requires EOL at EOF (e.g. git, svn) wouldn't warn you and throw an error;
  • Explicit (require last EOL) is better than implicit (allow both variants);
  • Diffs would contain less noise;
  • EOL at EOF also guarantees that every line in file will be ended with EOL that simplifies parsing a little;
  • Your variants here.

Cons:

  • Need to configure .editorconfig file and install plugin for your IDE (you working in notepad, right?)?

upd I'm working on Windows via putty (nano/mcedit) and in GUI (Sublime, earlier Intype/Idea/Netbeans/FAR) and OS X (Sublime/TextMate/terminal editors). And you really confusing me when saying "Not every Windows tool could be configured to use LFs" — can you tell me which one can't be configured?

@twirl
Copy link
Author

twirl commented Mar 9, 2015

POSIX tools that requires EOL at EOF (e.g. git) wouldn't warn you and throw an error;

I've never seen such a warning from git or other tools I use. I should mention that we develop 1M+ lines of code project and never have any problem with EOLs since we stopped appending one file to another with cat and started using specialized building tools four years ago.

Cons:
Need to configure .editorconfig file and install plugin for your IDE (you working in notepad, right?)?

What about "Plurality is not to be posited without necessity" rule?
This stupid rule comes from ancient days. Nobody really tested its necessity.
All the discussion lively reminds me famous experiment on monkeys — where finally nobody knows why really it's prohibited to do something, but still fiercely insists on keeping the rule.

@qfox
Copy link

qfox commented Mar 9, 2015

I vote for remove this rule from codestyle.

@zloylos Removing is even worse than an opposite. Here we should decide to use EOL at EOF or don't. Explicitly.

@tarmolov I know that Yandex codestyle is more common than BEM community, but canonical version has this rule and new one is based on Yandex jscs preset that also has this rule. It means that some other guys will vote for this rule (I don't know if it weights anything for ya), just fyi.

@qfox
Copy link

qfox commented Mar 9, 2015

I've never seen such a warning from git or other tools I use.

It's not a problem ;-) Now you see:
image

and never have any problem with EOLs since we stopped appending one file to another with cat and started using specialized building tools four years ago.

I will agree with you when git will stop noticing me about this. I know just 99% of tools can handle this. But it's free to add this line automatically, so no reason to make git sad.

What about "Plurality is not to be posited without necessity" rule?
This stupid rule comes from ancient days. Nobody really tested its necessity.
All the discussion lively reminds me famous experiment on monkeys — where finally nobody knows why really it's prohibited to do something, but still fiercely insists on keeping the rule.

Yep. And nope. See above. Risk of fail vs free adding EOL at EOF by editor — your choice is here.

Personally I really hate this message in git, and moreover I hate diffs that can't be resolved automatically at the EOF without EOL at EOF. That's my practical experience why we must use this rule. And it's in git. I don't even want to say about svn's conflict resolver...

@ikokostya
Copy link
Contributor

This rule helps us keep our sources in same style and removes redundant git diffs. Of course you need setup your editor, but only once.

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

No branches or pull requests

8 participants