Add style guide #1894

Theodus · 2017-05-08T18:09:35Z

This PR adds a style guide for the standard library and re-formats the standard library code to conform to it.

[skip ci]

Theodus · 2017-05-08T18:10:26Z

This PR should not be merged until we agree about the contents of the style guide and re-format the code.

jemc

Awesome work!

Thanks for taking the time to put this together!

jemc · 2017-05-08T19:47:34Z

STYLE_GUIDE.md

I personally disagree about this syntax for multiline arrays - my preferred style is:

let ns = [as USize: 1 2 3 ]

However, I think @sylvanc cancels me out on this 🙈

I like @jemc style

Sylvan likes the opposite of Joe.

Sean doesn't care that much. Sean is good with whatever.
I suggest rock, paper, scissors between Theo and Joe.

I really prefer the one in the guide, as it matches other functional languages (F#, OCaml, ML) and the middle one especially (because the brackets line up).

jemc · 2017-05-08T19:48:43Z

STYLE_GUIDE.md

+
+  // Not OK
+  { (a: USize, b: USize): USize => a + b }
+  ```


I think we need to give some guidance on multiline lambdas as well, though I'd be happy to add it as a followup PR if you wanted to go ahead and merge this without that detail.

More examples for this have been added.

jemc · 2017-05-08T19:50:15Z

STYLE_GUIDE.md

+  if cond then e1 else e2 end
+  ```
+
+- Match expression cases are __not__ indented.


This can be a bit misleading - it might be read as implying that match expression cases with multiple lines of body should not be indented, and I think we agree that they absolutely should.

We probably need an example showing this.

Another example for this has been added.

jemc · 2017-05-08T23:17:04Z

STYLE_GUIDE.md

+  // Not OK
+  {(a: USize, b: USize): USize =>
+    a + b
+  }


Actually, this "Not Ok" example would be my preference, especially when considering lambdas that span many lines.

But I'm curious to hear what @sylvanc thinks, since the recommended "Ok" style here is similar to what @sylvanc prefers for array literals.

Either way, it should be consistent between lambdas, array literals, union types, etc.

I think I disagree about that - I don't think a lambda is in the same category as the other two.

In part because it will be quite common for lambdas to become rather complex by comparison, with other levels of nested indentation. Arrays can technically contain arbitrarily complex blocks of code, but in practice I think any nested levels of indentation would be quite rare. In contrast, nested levels of indentation are quite common in lambdas.

Here's an example of a lambda that isn't really that complex conceptually, but due to the nested levels of indentation, using the "same line" closing bracket strategy would look quite awkward:

rs.next({(p: zmq.SocketPeer, m: zmq.Message)(rs) => if m.size() > 0 peers = state.peers() for peer in peers.values() peer.send(recover zmq.Message.>append(m).>push("S") end) end end }

Just imagine how it would look with a large match block thrown in.

I'd argue that it's more important for lambda to be stylistically consistent with other constructs that are large blocks of sequential code, like if, while, for, etc.

SeanTAllen · 2017-05-10T19:36:23Z

STYLE_GUIDE.md

+- Blank lines between __one-line__ functions or primitives may be omitted if they are related.
+
+  ```pony
+  primitive Red    fun apply(): U32 => 0xFF0000FF


unless alignment is part of the spec, i think this example shouldn't have the lining up

cquinn · 2017-05-12T04:51:18Z

STYLE_GUIDE.md

+
+### Line Length
+
+The maximum line length is 80 columns.


Has there been any discussion of widening this limit to 100 columns? While I understand not letting lines get too wide, 80 always seems so narrow to me. I find 100 to be a sweet spot.

Outside of the stdlib, that's fine of course. For the stdlib, 80 works everywhere without line wrapping problems. This is very useful on some displays - and even more often when I want to display two files side by side on a laptop :)

I do agree that a modest max column limit makes the code easier to read (less blank area) and easier to compare side by side. But do you really have displays that you use for this that can't show 100x2 columns?

I just wanted to get my 2¢ in before it was too late, but I'll stop whining if no one else prefers 100.

I use only half of my laptop display for the text editor, so I can usually see about 76 columns without scrolling :)

80! and yes my display does not do 100x2 with the font size i find comfortable.

OK, 80 it is. I've given in and always wear my readers when on the computer, which then lets me use a 10pt font on my retina 15" MacBook Pro to get about 300 columns across the screen. But I guess I'm an outlier.

cquinn · 2017-05-12T04:53:26Z

STYLE_GUIDE.md

+
+### Blank Lines
+
+- One blank line is placed between top-level declarations such as classes, primitives, actors, etc.


I find that having the same one-line gap between functions and classes can cause the classes to get lost a bit. Especially since there is no end on functions and classes.

cquinn · 2017-05-12T04:57:20Z

STYLE_GUIDE.md

+end
+```
+
+### Function Declarations


Shouldn't this be Function Definitions? Or is that just a C/C++ distinction?

Should we also mention that this is for new and be as well?

Maybe say "method" instead of "function"? We use method to mean new/fun/be, generally. Definition vs declaration is a distinction in many languages (not just C/C++), but not in Pony.

cquinn · 2017-05-12T05:01:52Z

STYLE_GUIDE.md

+  ```
+
+- Multiline function parameters are each listed on a separate line. The return type preceded by a `:` is placed at the same indentation level followed by a question mark (if the function is partial). The `=>` is placed on a separate line between the parameter list and the function body at the same indentation level as the `fun` keyword.
+


What about a case where the parameters all fit on one line, but then the return type has to be wrapped? Would something like this be OK?

fun tag _init(typ': ValueType, default': (Value | None)) : (ValueType, Value, Bool) ? => // ...

Good example. That's exactly what I do :)

That is what we do in the sendence code base as well

Example added

cquinn · 2017-05-12T05:07:36Z

STYLE_GUIDE.md

+### Whitespace
+
+- Use spaces around binary operators and after commas, colons, and semicolons. Do not put spaces around brackets or parentheses.
+


What about the = used to defined default parameter values?

Spaces around those too please :)

Theodus can you update to include that?

Please no force push that will kill off comments early though.

Example added

patternspandemic · 2017-05-12T19:26:43Z

Here are a couple areas of the style guide I thought could use some clarification, as I came across styling some code I wrote:

Multi-line lambdas with capture lists.
Multi-line type declarations with nested type parameters (i.e. JsonObject data)
Long match expression cases of the following form. (Here I decided to break after the | and align "string6" under "string1" to group the set, and to not confuse a match | with the or | of the expression.)

match s
| "string1" | "string2" | "string3" | "string4" | "string5" |
  "string6" | "string7" | "string" => Some()
| "other" => Other()
end

sylvanc

This is awesome and @Theodus is awesome.

sylvanc · 2017-05-17T19:33:31Z

STYLE_GUIDE.md

I really prefer the one in the guide, as it matches other functional languages (F#, OCaml, ML) and the middle one especially (because the brackets line up).

sylvanc · 2017-05-17T20:01:54Z

STYLE_GUIDE.md

+
+### Line Length
+
+The maximum line length is 80 columns.


Outside of the stdlib, that's fine of course. For the stdlib, 80 works everywhere without line wrapping problems. This is very useful on some displays - and even more often when I want to display two files side by side on a laptop :)

sylvanc · 2017-05-17T20:05:19Z

STYLE_GUIDE.md

+- Fields have no blank lines between them.
+- Functions are separated from other members by one line.
+
+  ```pony


These all need indentation :)

sylvanc · 2017-05-17T20:09:18Z

STYLE_GUIDE.md

+### Whitespace
+
+- Use spaces around binary operators and after commas, colons, and semicolons. Do not put spaces around brackets or parentheses.
+


Spaces around those too please :)

sylvanc · 2017-05-17T20:11:50Z

STYLE_GUIDE.md

+
+  ```pony
+  if cond1 then e1
+  elif cond2 then e2


sylvanc · 2017-05-17T20:12:19Z

STYLE_GUIDE.md

+
+  if cond1 then
+    e1
+  elif cond2 then


sylvanc · 2017-05-17T20:23:16Z

STYLE_GUIDE.md

+end
+```
+
+### Function Declarations


Maybe say "method" instead of "function"? We use method to mean new/fun/be, generally. Definition vs declaration is a distinction in many languages (not just C/C++), but not in Pony.

sylvanc · 2017-05-17T20:24:07Z

STYLE_GUIDE.md

+  ```
+
+- Multiline function parameters are each listed on a separate line. The return type preceded by a `:` is placed at the same indentation level followed by a question mark (if the function is partial). The `=>` is placed on a separate line between the parameter list and the function body at the same indentation level as the `fun` keyword.
+


Good example. That's exactly what I do :)

sylvanc · 2017-05-17T20:26:27Z

@Theodus this is fantastic. Thank you!

@patternspandemic good call!

Fixes formatting of I32.min_value(). TODO: the u32 and u64 funs in _format_int.pony could probably be largely re-unified. Update formatting to follow style guide proposal (ponylang#1894). Fixes: ponylang#1920

SeanTAllen · 2017-05-25T12:43:17Z

STYLE_GUIDE.md

+    offset: USize = 0,
+    nth: USize = 0,
+    predicate: {(box->A!, box->A!): Bool} val =
+      {(l: box->A!, r: box->A!): Bool => l is r })


this wrapping seems very, weird

To me it seems that this is the best way to be consistent with the "hanging" style for assignments with a large right-hand-side. That is, a parameter with a large default value should have the same style as such an assignment.

cant say i disagree with that. it still seems a little weird.

Fixes formatting of I32.min_value(). TODO: the u32 and u64 funs in _format_int.pony could probably be largely re-unified. Update formatting to follow style guide proposal (ponylang#1894). Fixes: ponylang#1920

Fixes formatting of I32.min_value(), I16.min_value(), etc. TODO: the various type-specific funs in _format_int.pony could probably be largely re-unified. Update formatting to follow style guide proposal (ponylang#1894). Fixes: ponylang#1920

[skip ci]

jemc

As discussed on the sync call, I'm relenting on multiline arrays.

Thanks for tackling this, @Theodus!

Theodus added do not merge This PR should not be merged at this time needs discussion during sync labels May 8, 2017

Theodus mentioned this pull request May 8, 2017

Create Style Guide #1775

Closed

jemc reviewed May 8, 2017

View reviewed changes

cquinn mentioned this pull request May 10, 2017

Initial CLI package implementation w/ tests and updated examples #1897

Merged

SeanTAllen reviewed May 10, 2017

View reviewed changes

cquinn reviewed May 12, 2017

View reviewed changes

SeanTAllen mentioned this pull request May 12, 2017

Add Pony style guide WallarooLabs/wally#514

Closed

sylvanc suggested changes May 17, 2017

View reviewed changes

SeanTAllen mentioned this pull request May 24, 2017

Customize format for I32/U32 (issue #1920) #1927

Merged

SeanTAllen reviewed May 25, 2017

View reviewed changes

Add style guide

cbb756f

[skip ci]

Theodus removed the needs discussion during sync label Jun 7, 2017

jemc approved these changes Jun 7, 2017

View reviewed changes

sylvanc approved these changes Jun 7, 2017

View reviewed changes

fmt stdlib

c632047

Theodus removed the do not merge This PR should not be merged at this time label Jun 7, 2017

Theodus merged commit 07763fa into ponylang:master Jun 8, 2017

Theodus deleted the style-guide branch June 8, 2017 00:46

+
+
+
+                ]


		### Blank Lines

		- One blank line is placed between top-level declarations such as classes, primitives, actors, etc.

		```

		- Multiline function parameters are each listed on a separate line. The return type preceded by a `:` is placed at the same indentation level followed by a question mark (if the function is partial). The `=>` is placed on a separate line between the parameter list and the function body at the same indentation level as the `fun` keyword.

		### Whitespace

		- Use spaces around binary operators and after commas, colons, and semicolons. Do not put spaces around brackets or parentheses.

Add style guide #1894

Add style guide #1894

Conversation

Theodus commented May 8, 2017

Theodus commented May 8, 2017

jemc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jemc May 9, 2017 • edited Loading

Choose a reason for hiding this comment

jemc May 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patternspandemic commented May 12, 2017

sylvanc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sylvanc commented May 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jemc left a comment

Choose a reason for hiding this comment

jemc May 9, 2017 •

edited

Loading

jemc May 9, 2017 •

edited

Loading