Skip to content

Conversation

yglukhov
Copy link
Member

@yglukhov yglukhov commented Jun 8, 2018

No description provided.

@data-man
Copy link
Contributor

data-man commented Jun 8, 2018

Yes, the stdlib not everywhere complies with this rule. :)

@dom96
Copy link
Contributor

dom96 commented Jun 8, 2018

If you relax this rule then there will be even less reason for it to be followed.

To me 80 character line limit is a good size, but it seems that you want to change this size for your code, so why not propose a higher limit instead?

@Araq
Copy link
Member

Araq commented Jun 9, 2018

It's ridiculous to discuss this further, I'm going to save everybody's time.

@Araq Araq merged commit d454350 into nim-lang:devel Jun 9, 2018
@dom96
Copy link
Contributor

dom96 commented Jun 9, 2018 via email

@dom96
Copy link
Contributor

dom96 commented Jun 9, 2018

Sorry, but I reverted this. ae342f8

@dom96
Copy link
Contributor

dom96 commented Jun 9, 2018

Happy to discuss this further here or somewhere else.

@cheatfate
Copy link
Member

Ok this is article which explains why 80 characters per line is better
http://katafrakt.me/2017/09/16/80-characters-line-length-limit/

@yglukhov
Copy link
Member Author

Firstly, the goal of this PR is not to figure whether 80chars is good or bad. I'm fully aware there are lots of 80chars adepts, and this is no place for another holy war.

However it's just not fair to have a strict rule in the officially blessed NEP with this rule broken all over the place in the official nim codebase for years. This just demonstrates how unimportant this rule is among nim contributors/maintainers community, but for some weird reason all nim users are still advised to strictly follow it, otherwise they are not "NEP-1" compliant :).

@cheatfate
Copy link
Member

Nim's standard library has just 1.77% of non-NEP1 lines. So it demonstrates that mostly Nim contributors folllow NEP1. And this is not the holy war, its just a rule which is pretty common and standard in software development.

@dom96
Copy link
Contributor

dom96 commented Jun 10, 2018

Unless we have travis testing for NEP-1 compatibility there will always be some piece of code that doesn't follow it. Because of that I don't think this is a valid argument to change NEP-1.

@yglukhov yglukhov deleted the nep1.1 branch June 10, 2018 13:25
@zah
Copy link
Member

zah commented Jun 11, 2018

The long-term effect of having a strict rule in NEP-1 is that it's going to be adopted by linters. Then every violation will become an unnecessary warning.

@yglukhov
Copy link
Member Author

Imo non-NEP lines percentage has nothing to do with NEP. My nimx lib is 95% compliant, even though I don't care about the 80chars at all. I'm pretty sure most of the code in the world will be somewhere around 95% compliant, even though it might not be restricted to 80chars. As such I consider @cheatfate's metrics to be somewhat flawed because the percentage should be measured not against the total LOC but against the number of manually wrapped lines.

Now 80chars limit often makes the remaining code unnaturally shorter (hard-wrapped), instead of being logically split to multiple logical statements. Such hard-wrapping is usually no better than auto soft-wrapping in the text editor (soft wrapping in modern text editors is pretty configurable. indentations, max length, you name it). As such this hard-wrapped text is not universal in regards to viewer's preferences (e.g. on a big monitor i'm fine with up to 150chars). But what's most unfortunate such hard wrapped code becomes almost unreadable when it's restricted to less than 80chars (say, 60-70 chars if you open 3 vertical panes on 13" monitor), and at this point I would be a lot happier with soft-wrapping only.
One last argument to soft wrapping is that you can train your brain to read the code soft-wrapped by your text editor, as it is done uniformly. Reading differently manually wrapped text is more of a pain if you think of it.

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

Successfully merging this pull request may close these issues.

6 participants