nim-lang / RFCs

A repository for your Nim proposals.
136 stars 23 forks source link

nimpretty should keep proc declaration as 1 line instead of breaking into multiple lines #189

Open timotheecour opened 4 years ago

timotheecour commented 4 years ago

nimpretty formats as:

proc toOpenArray*[I, T](x: array[I, T]; first, last: I): openArray[T] {.
  magic: "Slice".}
proc toOpenArray*(x: string; first, last: int): openArray[char] {.
  magic: "Slice".}

I find the following more readable:

proc toOpenArray*[I, T](x: array[I, T]; first, last: I): openArray[T] {.magic: "Slice".}
proc toOpenArray*(x: string; first, last: int): openArray[char] {.magic: "Slice".}

even if it goes beyond the col limit. It also works better with tooling, eg grep etc would show the full proc declaration instead of a partial one.

I also find the 80 col limit rather archaic, I'd personally prefer no col limit (all modern editors should support soft line wrap), but would be ok with increasing it to 120 as a compromise.

links

Clyybber commented 4 years ago

I think we should increase the limit to 120.

disruptek commented 4 years ago

Your "more readable" example instantiates a scrollbar on my display. I would personally sooner reduce the line length rather than raise it. Yes, that's ridiculous.

If you're concerned about grep, maybe you should close your "alias" PR also.

SolitudeSF commented 4 years ago

relying on code formatters is a mistake

Araq commented 4 years ago

I have no opinion on this.

c-blake commented 4 years ago

nimpretty --maxLineLen:128 (or whatever!) already works fine. So, this is literally only a debate about defaults which seems..maybe not worth a whole RFC?

Maybe nimpretty should read a config file where you can change that default just once (along with --indent, maybe). That seems like an easy PR that would meet little resistance. You'll never get everyone to agree on everything. If anything nimpretty may be expected to grow more options over the years. The GNU indent program has a couple of dozen settings for formatting C code. Just an attempt at a constructive remark.

c-blake commented 4 years ago

I should have said the extent to which it is not a debate about defaults, it's about some new --bias-to-keep-def-pragmas-on-one-line feature for nimpretty which would also benefit from some config file.

disruptek commented 4 years ago

The point of such tools is to remove options, not to add them.

c-blake commented 4 years ago

I think that is a matter of perspective. Some people like them to clean up their sloppy, heat of the moment formatting. I doubt some opt-in nimpretty --maxPragmaLine is really a slippery slope to disaster or would bother anyone one who doesn't want it.

One can only partially remove the dizzying possibilities of "malformatted code" and still have many somewhat arbitrary choices left over. Nim's overall philosophy is to enable many styles. Hence options. That said, I don't use these things myself and agree with @SolitudeSF.

Regardless, nimpretty already has two options. Why make people type them on the command-line every time or alias/wrap? Maybe there's already some issue asking for this feature, but my strace didn't reveal the program looking for any config file.

timotheecour commented 4 years ago

nimpretty --maxLineLen:128 (or whatever!) already works fine. So, this is literally only a debate about defaults which seems..maybe not worth a whole RFC? Maybe nimpretty should read a config file [...]

this RFC is indeed about the default used, as it affects most nim code out there (in particular stdlib/compiler sources), and (IMO) would make sources easier to read/write/edit/search. Having a config file is a separate issue and wouldn't help with this.

c-blake commented 4 years ago

Ok. Well, if that's all this is, I am a +1 vote on 80/keeping the default the same then. I don't see why anyone outside the compiler/stdlib would be a prisoner of the default (especially with a config file). Much of the compiler/stdlib seems to have not been run through nimpretty. A quick grep -r '^..81dots*' even gives 9% lines longer than 80. So, even that is hardly hyper strict.

c-blake commented 4 years ago

Apologies - I did not restrict to only .nim files. Doing that it's only 8124./1259786 over-80 lines or 0.65% violations. Still a lot, though.

timotheecour commented 4 years ago

Much of the compiler/stdlib seems to have not been run through nimpretty

there were reformatting PR's that fed select nim files through stdlib; which makes sense btw (and saves reviewer time, making formatting reviewer comments obsolete), once nimpretty is stable enough (and with good enough default, thus this debate here), deferring formatting to tooling makes sense (note that nimpretty offers on/off escape hatch syntax in the rare case where manual formatting for a block of code should override nimpretty default). So eventually you may expect all of compiler/stdlib to be fed through nimpretty. In the meantime, nimpretty --difflines (analog of git clang-format, see https://github.com/nim-lang/Nim/issues/8458#issuecomment-457055195) would be great to have to keep reformatting limited to the diffed lines.

c-blake commented 4 years ago

Makes sense to do it incrementally/as needed. Sorry to disagree about "archaic" limit of 80, but with wider aspect ratios some of us moved to two side-by-side 80 column terminals 20 years ago while others might be in portrait mode on phones. 80 is a pretty time honored standard. Human eyesight hasn't changed much..If anything the diversity of aspect ratios has objectively proliferated making agreement harder and so venerable standards stickier (rightly or wrongly people often defer to tradition when there is no clearly right choice).

I feel like making nimgrep better might be a better way for you to get search satisfaction. Since it's Nim-specific you could have nimgrep --wholeDecl procName to get the whole declaration or something. Works even for long argument lists! Visual satisfaction while reading, well, that you might not get without serious editor magic (reformatting, being clever about what you dirty, reformatting back, etc.).

Araq commented 4 years ago

nimpretty --difflines (analog of git clang-format, see nim-lang/Nim#8458 (comment)) would be great to have to keep reformatting limited to the diffed lines.

Yeah, since then I changed my mind and --difflines seems to be a good idea indeed. And not even all that hard to implement.