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

Allow leading logical operators #5279

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

helixbass
Copy link
Collaborator

@GeoffreyBooth re-opening against master

See original PR: #5068

Copy link
Contributor

@edemaine edemaine left a comment

Choose a reason for hiding this comment

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

This old PR looks nice too. I recently ran into this issue when editing the CoffeeScript source, and wanting to write:

if (really_long_line
    or another_long_line) and ...

(because the or didn't fit on the first line). I think it'd be nice for this to be equivalent to

if (really_long_line or
    another_long_line) and ...

which this PR does.

@GeoffreyBooth
Copy link
Collaborator

This one is fine too, it just needs a rebase.

@STRd6
Copy link
Contributor

STRd6 commented Apr 4, 2022

I like this proposal, let me know if there is anything I can do to help.

@helixbass
Copy link
Collaborator Author

Hi guys, sorry I've been MIA on Coffeescript stuff, mostly been focusing on other things

I merged main into this branch and see bin/cake test passing locally but CI appears to be failing on git diff --exit-code, is there a different set of local rebuilds I should be doing to get "matching" lib/*.js output (I was testing against and committed the output of git checkout lib && bin/cake build:except-parser && bin/cake build:parser)?

Also the details of this PR are a bit fuzzy but I'd tend to trust my past self 😜

@GeoffreyBooth
Copy link
Collaborator

Run cake release to fully update everything.

I put a note on the other version of this, is there a way to handle this without introducing a new rewriter pass? It feels like something that should be doable within the lexer.

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.

4 participants