Closed pchiusano closed 3 days ago
Good discussion —
On first read, I hated giving foo()
a different meaning from foo ()
, it feels hard to explain.
Generalizing it to multi-arg calls, introducing a full, second, function-calling syntax,
<id> '(' [args,..] ')'
vs
<id> [args..]
is much better IMO than introducing a single, special case.
The fact that the pretty-printer uses both forms though means that everyone has to learn both forms.
So, I think it's an improvement, but it's worth thinking about the end-game too, to know which direction we should be heading in, and would like to hear other feedback.
Okay, after hearing the discussion, I'm going to axe the general function application syntax for now (which many thought was iffy given the pretty-printer doesn't use it) and just have the PR be about replacing !foo
with foo()
. Thank you very much to everyone who weighed in.
Note that changing function call syntax to C-style more generally is a bigger deal. I don't think it plays nicely with the rest of Unison's syntax, its use of layout, curried functions, etc, and I consider it out of scope for this PR, more something we'd consider as part of a comprehensive rethink of Unison's syntax. Or if we were supporting #499. So, not now. :)
@aryairani one note on your comment above, there actually isn't a consistent rule between the two -
foo()
means (foo ())
with high precedence.foo(x,y)
means (foo x y)
, aka left nested function application where the comma separated elements are the list of args.The base case of the second interpretation would have that foo()
means the same as foo
(it's a left fold starting with foo
, with the empty list of arguments), not that it's passing Unit
to foo
. You'd need to write foo(())
to get (foo ())
without a special case.
Aside: I think it could be cool to let users supply their own syntax sugar since then it's opt in, but I tend to agree it's a little weird and possibly confusing to add sugar for everyone when that isn't going to be used by the pretty-printer.
I'll get this cleaned up and all the transcripts passing.
@pchiusano Oh, I see, thanks for pointing that out. I glossed over it, because it looks like a function with no args.
I guess I'm not convinced then that this special case helps, short of a comprehensive rethink, which we should consider undertaking. (A comprehensive rethink doesn't necessarily mean radical changes, but who knows.) I'm not sure who'd be best to lead it but I'd be happy to try.
I don't have any concerns about just eliminating the !
syntax though, calling foo ()
or bar (foo ())
or bar (force foo)
as needed.
K, this is all set, I reduced the scope as discussed. I ack that some people might prefer removing any special treatment for thunk forcing (or at least phasing it out of the pretty-printer). These things are always going to be a judgement call / personal preference which is okay. :)
Also just noting again how easy it is to change our minds on this anytime later. (For instance, we could easily drop the special case from the pretty-printer and have it print with an extra level of parens, and we could also drop the special case from the parser, forcing extra parens there as well.) This PR will not be the last time we get to bikeshed Unison's syntax. :)
Paul and I discussed on Slack, and decided to give this PR a shot; gambling that users will not, in reality, be confused by this change, even without requiring any immediate update to the language docs, since the parser continues to accept the old syntax.
Any "told-you-so"s will be dealt with on a case-by-case basis.
From recent discussions, there's a feeling that
!
isn't really paying its way as special syntax. It...()
to a function), and...!
I've already generally typed the symbol and have to backtrack in the buffer.!
, a loose convention used for a variant of a function that runs its effects rather than returning a thunk, so you sometimes have expressions likeobject.at! !qux (quaffle 1 2)
and it makes me sad to think about trying to explain why the!
appears at the end ofobject.at
but at the start of!qux
...This PR changes things. Before, you'd write
!foo
to meanfoo ()
with high precedence. Now you can just usefoo()
with no spaces after thefoo
to get the same precedence and meaning asfoo ()
. The pretty-printer uses this syntax as well. The old!foo
syntax is still supported in the parser but is now unused by the pretty-printer.Here's a few examples (which parse and pretty-print like this):
The lack of any space after
Environment.default
andpool
makes this bind tighter than the normal function application syntax. If you put any spaces, the parenthesized thing is treated like a tuple, just like now.(possibly controversial, ended up being removed from this PR, but including for posterity) It was easy to generalize this syntax to have it work for higher-precedence function application, just like in C, Java, etc:
foo sqrt(2.0)
parses asfoo (sqrt 2.0)
foo max(x, y)
parses asfoo (max x y)
The pretty-printer still prefers the
foo (max x y)
syntax in these cases though, so consider this more of a "I'm typing and don't feel like going backwards to add parens" or "I forgot that I'm not programming in Java, but this still parses the way Java would".Implementation notes
A simple, surgical change to the term parser and pretty-printer.
Interesting/controversial decisions
Possibly this whole thing is controversial. :)
Some possibilities I considered:
foo(x, y, z)
for function application, instead just supportingfoo()
for forcing. I didn't see the harm in including the more general syntax. I'll probably use it myself when I'm feeling lazy about going back and adding parens, and it might make folks coming from other languages more comfortable. (I have seen people accidentally usingfoo(x,y,z)
for function application many times.)blah foo() arg2
in the pretty-printer - print this asblah (foo ()) arg2
. So basically - this PR, but don't use the syntax in the pretty-printer. I don't like this as much. I feel likeforkAt pool() do ...
is pretty easy to parse even if you don't know Unison. You tend to assume thatpool()
!
syntax entirely. I'd rather keep it around for a while. We can remove it after people have adjusted and we're confident in the change.This is also easy to back out anytime later.
Test coverage
Existing transcripts provide pretty good coverage, and I added a round trip test. I think coverage is good.
Loose ends
None