Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Arrays as sequences #1741

Merged
merged 3 commits into from Mar 24, 2017
Merged

Arrays as sequences #1741

merged 3 commits into from Mar 24, 2017

Conversation

sylvanc
Copy link
Contributor

@sylvanc sylvanc commented Mar 23, 2017

Previously, a comma ',' was used to separate array elements. In Pony, a
comma is the tuple operator, and as a result, an array appeared to be
composed of a tuple of elements.

This change defines an array as a sequence of elements instead. As a
result, no separator is needed when elements are on new lines, or a
semicolon ';' (i.e. the sequence operator) can be used when elements are
on the same line.

This change means that all instances of ',' in Pony indicate a tuple
(including, logically speaking, parameter lists, arguments, type
arguments, etc.) and all instances of ';' indicate a sequence.

Resolves #1736.

Previously, a comma ',' was used to separate array elements. In Pony, a
comma is the tuple operator, and as a result, an array appeared to be
composed of a tuple of elements.

This change defines an array as a sequence of elements instead. As a
result, no separator is needed when elements are on new lines, or a
semicolon ';' (i.e. the sequence operator) can be used when elements are
on the same line.

This change means that all instances of ',' in Pony indicate a tuple
(including, logically speaking, parameter lists, arguments, type
arguments, etc.) and all instances of ';' indicate a sequence.
@SeanTAllen
Copy link
Member

SeanTAllen commented Mar 23, 2017

I'd like to hold on merging this until 0.11.4 is released so that it can go out without any breaking changes as that release is mostly about testing the new release scripts.

#1738

"\t"
Format.int[U64](nspo where width=10)
" ns/op"
]
Copy link
Member

@jemc jemc Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick style point to think about and discuss.

How would you feel about using the following bracket style for multi-line arrays in the standard library?

let sl = [
  name
  "\t"
  Format.int[U64](ops where width=10)
  "\t"
  Format.int[U64](nspo where width=10)
  " ns/op"
]

Quite subjective, I know, but it feels more balanced to me, and also more consistent with the style being used for lambda curly braces so far.

@@ -99,14 +99,14 @@ class iso _TestFilter is UnitTest
fun name(): String => "files/Glob.filter"

fun apply(h: TestHelper) ? =>
let m = Glob.filter([ "12/a/Bcd", "12/q/abd", "34/b/Befd"], "*/?/B*d")
let m = Glob.filter([ "12/a/Bcd"; "12/q/abd"; "34/b/Befd"], "*/?/B*d")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it wasn't introduced in this PR, but this seems like a good opportunity to clean up the style inconsistency here for the space following the [, which isn't consistent with other single-line arrays in the standard library.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure :)

@jemc
Copy link
Member

jemc commented Mar 23, 2017

Also, does this need a change to pony.g to update the public grammar?

@killerswan
Copy link
Member

Just FYI, if you've edited the changelog, you may see conflicts because we've now cut a 0.11.4 release. 😅

@sylvanc sylvanc added the changelog - changed Automatically add "Changed" CHANGELOG entry on merge label Mar 24, 2017
@sylvanc
Copy link
Contributor Author

sylvanc commented Mar 24, 2017

Yes, needed a pony.g update :)

I'm afraid I'm a fan of the "leading delimiter on a line" approach. I suppose it's more ML/OCaml/F#, whereas "trailing delimiter on a line" is more C/C#/Java, so I can't really say my way is better, just that I'm a fan of it.

I particularly like it when not binding to a variable, i.e.:

fun greetings(): Array[String] =>
  [ "hi"
    "hello"
    "aloha"
  ]

And this extends to pipelining (not yet implemented of course), etc.

@jemc
Copy link
Member

jemc commented Mar 24, 2017

I'm afraid I'm a fan of the "leading delimiter on a line" approach

I'm afraid, too! 😱

At any rate, I agree that it's subjective, and neither option is intrinsically better than the other in an absolute sense. It just makes me twitch a little bit. 😰

@killerswan
Copy link
Member

Mwahahaha:

fun greetings(): Array[String] =>
  [
    "hi"
    "hello"
    "aloha"
  ]

Don't mind me.

@Theodus
Copy link
Contributor

Theodus commented Mar 24, 2017

@killerswan The madman!

@sylvanc sylvanc merged commit 73cb823 into master Mar 24, 2017
ponylang-main added a commit that referenced this pull request Mar 24, 2017
kamilchm added a commit to kamilchm/pony-stable that referenced this pull request Mar 30, 2017
jemc pushed a commit to ponylang/pony-stable that referenced this pull request Mar 30, 2017
@SeanTAllen SeanTAllen deleted the array-as-sequence branch April 6, 2017 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - changed Automatically add "Changed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants