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

Check for stripMargin when adding | #904

Merged
merged 2 commits into from Sep 9, 2019

Conversation

@tgodzik
Copy link
Collaborator

commented Sep 6, 2019

Fixes #893

@tgodzik tgodzik requested a review from marek1840 Sep 6, 2019
): Boolean = {
var methodIndex = stringTokenIndex + 1

while (tokens(methodIndex).isWhiteSpaceOrComment ||

This comment has been minimized.

Copy link
@olafurpg

olafurpg Sep 6, 2019

Member

Does this handle interpolated strings?

s"""
  |Hello $world
""".stripMargin

This comment has been minimized.

Copy link
@tgodzik

tgodzik Sep 7, 2019

Author Collaborator

Yes, however I don't think there is a test case for it. I will add that.

This comment has been minimized.

Copy link
@gabro

gabro Sep 7, 2019

Member

I just tried the latest snapshot and the | addition didn't work for me when using an interpolator. Does this PR fix it?

This comment has been minimized.

Copy link
@tgodzik

tgodzik Sep 7, 2019

Author Collaborator

Och, I think there might be an issue actually coming to think of that 🙁 Will add test cases and fix it here.

This comment has been minimized.

Copy link
@tgodzik

tgodzik Sep 9, 2019

Author Collaborator

Ok, now I fixed it. I just assumed we tested it.

isMultilineString(sourceText, token) =>
new TextEdit(range, indent(sourceText, pos) + "|")
tokens.flatMap { tokens: Tokens =>
val tokenIndex = tokens.indexWhere {

This comment has been minimized.

Copy link
@marek1840

marek1840 Sep 9, 2019

Collaborator

Just a though, no need to act on it.

it would be nice to have the tokens as something akin to the iterator:

tokens.skipUntil{
 case token: Constant.String => (...)
 case _ =>  false
}

the isFinishedByStripMargin implementation would be much nicer

This comment has been minimized.

Copy link
@tgodzik

tgodzik Sep 9, 2019

Author Collaborator

This would imply that Tokens are mutable underneath or I would need to create a mutable structure over it :/ I prefer to keep any mutable state local and explicit.

This comment has been minimized.

Copy link
@tgodzik

tgodzik Sep 9, 2019

Author Collaborator

And I needed to change it even more to make it work with interpolated strings

@tgodzik tgodzik merged commit 9a88377 into scalameta:master Sep 9, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
scalameta.metals Build #20190909.2 succeeded
Details
@tgodzik tgodzik deleted the tgodzik:check-for-strip-margin branch Sep 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.