rescript-lang / syntax

ReScript's syntax as a standalone repo.
MIT License
256 stars 38 forks source link

light weight poly var encoding #76

Closed bobzhang closed 3 years ago

bobzhang commented 4 years ago

instead of #\"ease-in", allow users to have #"ease-in" in expressions and patterns

jfrolich commented 4 years ago

personally I'd prefer a @bsAs annotation, just like records in the cases where you'd like to bind to some arbitrary string so you can give the poly variant a nice name that gives a better user experience to consumers of the binding.

bobzhang commented 4 years ago

@jfrolich bs.as wont work since it's structural, it wont be first class

jfrolich commented 4 years ago

Ah I see! Makes sense.

bobzhang commented 4 years ago

@IwanKaramazow could this get in the first release?

jfrolich commented 4 years ago

Isn't the #\"ease-in" notation better? That notation works everywhere. Is it worth special casing it for poly vars to save one character? Would be another thing that people need to learn.

cknitt commented 4 years ago

If we can not get #easeIn [@bs.as "ease-in"], then I would definitely prefer #"ease-in" to #\"ease-in". I think the latter would drive me crazy after a while (especially when working on a German keyboard. :wink:).

bloodyowl commented 4 years ago

IMO #\"ease-in" is the simplest as it's already how escaping keywords work in the rest of the syntax. You could intuitively try it by guessing based on the rest of the language, whereas #"ease-in" looks like an entirely new syntax.

jfrolich commented 4 years ago

An idea. How about we pass an option to externals to convert certain poly vars to different strings. A bit like how we now have bs.string. The only reason why we need string representation to contain dashes or something like that, is when binding to JavaScript libraries. Then we don't need this notation at all in practice.

evxn commented 4 years ago

An idea. How about we pass an option to externals to convert certain poly vars to different strings. A bit like how we now have bs.string. The only reason why we need string representation to contain dashes or something like that, is when binding to JavaScript libraries. Then we don't need this notation at all in practice.

In practice we're using genType instead of externals ¯\_(ツ)_/¯

cknitt commented 4 years ago

If we can not get #easeIn [@bs.as "ease-in"], then I would definitely prefer #"ease-in" to #\"ease-in". I think the latter would drive me crazy after a while (especially when working on a German keyboard. 😉).

Also #"ease-in" would match very nicely with what @bobzhang proposes in https://github.com/BuckleScript/bucklescript/issues/4595#issuecomment-669645177.

We would then have:

.res:

#"ease-in"

.re/.ml:

`"ease-in"

😃

jfrolich commented 4 years ago

The new version of ReasonML is also adopting the # for poly variants, so this could make it exactly equal.

IwanKaramazow commented 4 years ago

Implemented in https://github.com/BuckleScript/syntax/pull/96

cknitt commented 4 years ago

@IwanKaramazow Why did you revert #96?

IwanKaramazow commented 4 years ago

@cknitt Sorry for the delay, I'm finishing a rewrite of the comment printing first. Will ship newer syntax features after that.

chenglou commented 4 years ago

@cknitt we've put this particular feature on pause; it's still under consideration, but there's some cost benefit analysis to be done here. There are quite a few syntax additions being planned and we do have a budget on the syntax complexity

cknitt commented 4 years ago

@IwanKaramazow @chenglou Thanks for the information. I would be glad if you could revisit this later on.

Things like #\"ease-in" and #\"while-editing" are really a pain to type (and read), and names like this do occur quite often in React Native.

chenglou commented 4 years ago

RRN definitely hits a pathologically often use of this feature unfortunately. But I'm not sure we should bent over backward for a single lib's usage of these (I say that hoping we don't go into a lengthy tangent on this issue. And you also know that I supervised the rewriting of RRN despite disagreements so it's not that I don't know the pain)

jfrolich commented 4 years ago

I don't think it's a big pain to write actually (only one extra character).

IwanKaramazow commented 3 years ago

This is now implemented in master: https://github.com/rescript-lang/syntax/pull/228. We weren't 100% sure of all the implications of this change. There's also the tradeoff of having yet another thing in the syntax. More syntax is just confusing and it bloats. Being a new Rust or c++ is not good syntax-wise. On the other hand poly-vars are essentially strings now, so this makes a great fit and will make interop easier.

jfrolich commented 3 years ago

Yes I kind of came around to it, it's pretty nice indeed!