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

Support unary operator spread over object literals #831

Closed
wants to merge 1 commit into from

Conversation

rhendric
Copy link
Collaborator

Livescript Compiled JavaScript
x = delete a{b, c: d}
var x, ref$;
x = {
  b: (ref$ = a.b, delete a.b, ref$),
  c: (ref$ = a.d, delete a.d, ref$)
};
x = do loaders{plugins, preferences: {server, client}}
var x;
x = {
  plugins: loaders.plugins(),
  preferences: {
    server: loaders.server(),
    client: loaders.client()
  }
};

Works almost exactly like unary spread over list literals/slices, but with that tasty object flavor.

I was originally going to support all unary operators, but then had a crisis of conscience when I looked at ^^{foo, bar} and asked whether that ought to be equivalent to x = {foo, bar}; ^^x or {^^foo, ^^bar}, or whether someone would freak out if typeof {} != typeof {foo:1} (although both of these issues exist with list spreads right now, so...). The somewhat arbitrary list of operators I decided to support is ~ !/not new do delete/delete! ++ --, but I'm not terribly attached to most of those decisions. The two examples demonstrated above are the ones I want to use.

Feedback please!

Works exactly like unary spread over list literals. As written, only a
subset of unary operators are supported, but this is easily amended.
@vendethiel
Copy link
Contributor

At that point, I think I'd prefer if there were an operator to trigger it.

@rhendric
Copy link
Collaborator Author

For object literals/slices only (inconsistent) or for list literals/slices also (breaking)?

FWIW, new {a}, do {a}, delete {a}, and the four inc/dec pre/post flavors of {a}++ (as well as more complex slicey versions of each) are all compiler errors right now; restricting to just those would result in minimal ambiguity.

Do we really want to live in a world where a{b, c} += 1 works, a[1, 2]++ works, but a{b, c}++ doesn't work, or needs an extra operator to trigger it?

@vendethiel
Copy link
Contributor

The breaking one.

@rhendric
Copy link
Collaborator Author

Hmm. How about if we marked the literal/slice to spread over with a splat?

new ...[circle, square] => [new circle, new square]
++...{level, max: max-level} => {level: ++level, max: ++max-level}
...player{strength, hp}-- => {strength: player.strength--, hp: player.hp--}

Extend to assignment operators?
...player{strength, hp} += 5 => {strength: player.strength += 5, hp: player.hp += 5}

@vendethiel
Copy link
Contributor

I'd say it looks consistent with what spread does -- but not sure how to parse it OTTOMH

@rhendric
Copy link
Collaborator Author

rhendric commented Feb 1, 2016

The parsing doesn't appear to be a problem; I bashed out a quick proof-of-concept. Is this proposal baked enough for me to harden the implementation into a new PR? Should I leave support for existing spread syntax intact, add a deprecation warning, or just remove it?

@vendethiel
Copy link
Contributor

Well, that's where I'd have liked @gkz's input.

I guess we can start to consider master to be LS2's head (maybe I'll create an actual LS2 dev branch later), so what I'd like to do before merging is just... Ask people!

If you could please post a message with a note of such a change in the LS2 threads. I hope we won't find any place where that's ambiguous (shouldn't be)

@dead-claudia
Copy link
Contributor

@vendethiel I see a branch prime for a push. The readme can be updated and posted on the new, replaced README. It only has one commit.

@vendethiel
Copy link
Contributor

No, not that branch. If anything, it should be deleted and recreated from current master.

@vendethiel
Copy link
Contributor

@rhendric I'm gonna go ahead and say "let's go for the explicit spread syntax". So long foo ...+...a<[b c d]> works

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.

3 participants