Skip to content

Fix #1372: Add handler for PatDefs to REPL #1436

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

Merged
merged 1 commit into from
Aug 8, 2016

Conversation

cswinter
Copy link
Contributor

@cswinter cswinter commented Aug 1, 2016

Adds a new handler to CompilingInterpreter to extract variable definitions from PatDef statements, which allows multiple assignment syntax and pattern match assignments to work properly in the REPL.

Review by @odersky?

@cswinter cswinter force-pushed the wip-repl-patdef-fix branch from df9080a to 2a17a7c Compare August 1, 2016 13:31
s""" + $fullPath.toString() + "\\n" """
} /* else */ {
"\"null\\n\""
}
}
Copy link
Contributor

@odersky odersky Aug 1, 2016

Choose a reason for hiding this comment

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

I find there are too many levels of escaping to be able to understand it. Can we try to put everything between one pair of """s and use only interpolation inside? That should also obviate the need for if_.

@odersky
Copy link
Contributor

odersky commented Aug 1, 2016

Otherwise LGTM

@cswinter cswinter force-pushed the wip-repl-patdef-fix branch from 2a17a7c to 57f41f6 Compare August 2, 2016 06:44

val resultExtractors =
for (varName <- boundNames)
yield resultExtractionCode(req, varName)
Copy link
Member

Choose a reason for hiding this comment

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

val resultExtractors = boundNames.map(name => resultExtractionCode(req, name)) is more concise

@cswinter cswinter force-pushed the wip-repl-patdef-fix branch 2 times, most recently from 31cf7f1 to 17ba942 Compare August 3, 2016 15:06
@smarter smarter merged commit 62348de into scala:master Aug 8, 2016
@smarter
Copy link
Member

smarter commented Aug 8, 2016

Thanks for making these changes, merged!

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