Skip to content

Extended parser to accept unicode forall and arrows#1847

Merged
paf31 merged 1 commit intopurescript:masterfrom
DavidLindbom:master
Jan 31, 2016
Merged

Extended parser to accept unicode forall and arrows#1847
paf31 merged 1 commit intopurescript:masterfrom
DavidLindbom:master

Conversation

@DavidLindbom
Copy link
Copy Markdown
Contributor

Saw it was mentioned in #766. This commit should now accept f ∷ ∀a m. Monad m ⇒ a → m a but also and in class definition resp. do notation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The notFollowedBy is unnecessary for these, no?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is a unicode symbol character? If so, it's still necessary.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh so you're saying that that may be used as a binding name?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, since we added unicode operators. @DavidLindbom could you please just verify that something like ←→ can be used as an operator (ideally in a test)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes they should work, just updated my test.

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Jan 23, 2016

Cool 😄 Could you please update CONTRIBUTORS.md too?

@garyb
Copy link
Copy Markdown
Member

garyb commented Jan 24, 2016

👍 for this 😄

@zudov
Copy link
Copy Markdown
Contributor

zudov commented Jan 24, 2016

Very cool, can't wait for this to get merged. But what about unicode lambda?

@garyb
Copy link
Copy Markdown
Member

garyb commented Jan 24, 2016

But what about unicode lambda?

Oh yeah, that'd be nice to have too.

@DavidLindbom
Copy link
Copy Markdown
Contributor Author

I will take a look at it later. I tried a similar quick fix but λ isn't a symbol but a identifier so I have to do some more work.

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Jan 24, 2016

I don't think we need a new lexeme. We can just use reserved to parse a lambda, surely?

@DavidLindbom
Copy link
Copy Markdown
Contributor Author

No reserved doesn't work because λa is a valid identifier. Lexing λa → a will result in Right ["λa",->,"a"] instead ofRight [,"a",->,"a"]`.

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Jan 24, 2016

Oh, that's a shame. I'm not sure what the best option is, in that case.

Perhaps we merge without λ, as you had it before, in 0.8, and revisit λ in 0.8.*. That would be my vote.

Another option is to use a space between the λ and the binder, but that's not going to look very good.

In either case, though, I think we can solve this without adding \ as a lexeme. Maybe a better solution is for char 'λ' *> notFollowedBy letter to be a lexeme.

@garyb
Copy link
Copy Markdown
Member

garyb commented Jan 24, 2016

Related discussion for the same thing in GHC: https://ghc.haskell.org/trac/ghc/ticket/1102

@DavidLindbom
Copy link
Copy Markdown
Contributor Author

@paf31 While I still think lambda would fit in to be a lexeme without cluttering it, I also agree that it would be best to revert to the previous commit and discuss later how to solve the lambda problem

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Jan 26, 2016

Could you please rebase, and I'll get this merged. Thanks.

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Jan 31, 2016

I'm sorry, by rebase I meant "and squash commits". Could you please do that so we can keep the commit log as simple as possible?

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Jan 31, 2016

Looks great, thanks! I'll wait for Travis and then get this merged in.

paf31 added a commit that referenced this pull request Jan 31, 2016
Extended parser to accept unicode forall and arrows
@paf31 paf31 merged commit 1b5d32d into purescript:master Jan 31, 2016
@texastoland texastoland mentioned this pull request Feb 28, 2016
@texastoland
Copy link
Copy Markdown

@DavidLindbom Would you be interested in showing this off in 5-10 minutes at the next online meetup? Are you on Twitter? Otherwise I can email.

@texastoland
Copy link
Copy Markdown

Maybe a better solution is for char 'λ' *> notFollowedBy letter to be a lexeme.

I had the same idea. Were there any downsides?

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.

6 participants