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

chore: Update Scalameta to 4.8.11 #5681

Merged
merged 2 commits into from
Oct 9, 2023
Merged

Conversation

tgodzik
Copy link
Contributor

@tgodzik tgodzik commented Sep 26, 2023

No description provided.

@tgodzik tgodzik force-pushed the update-scalameta branch 2 times, most recently from d3818e0 to fbf2412 Compare September 27, 2023 16:43
@tgodzik
Copy link
Contributor Author

tgodzik commented Sep 28, 2023

The changes in this pr inline some of the internal code from scalameta which is prone to change. This changed over the last few versions so if we try to compile older version of scala with an older version of scalameta this would break otherwise.

@tgodzik tgodzik marked this pull request as ready for review September 28, 2023 13:11
@@ -688,7 +688,6 @@ class ScalaToplevelMtags(
case WHITESPACE =>
nextIsNL()
case COMMENT =>
scanner.skipComment()
Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The method got removed and it's not necessary since getting the next token will also skip any comments

@@ -133,13 +133,13 @@ class JavaToplevelMtags(val input: Input.VirtualFile) extends MtagsIndexer {
parseToken
case _ =>
val token = kwOrIdent(
reader.charOffset - 1,
new StringBuilder().append(first).append(next)
reader.endCharOffset,
Copy link
Contributor

Choose a reason for hiding this comment

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

why did this offset change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was split into start and end in scalameta/scalameta#3339

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, but why did the -1 disappeared?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I think I removed it since the tests didn't pass, but maybe it should be just begCharOffset instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like begCharOffset works and that should be an equivalent of -1 before. Not sure why it worked with end offset, though I imagine our tests mostly use the end

This is an internal part of scalameta, which is not kept binary compatible, so it's safer to inline it.
@tgodzik tgodzik merged commit 2a57231 into scalameta:main Oct 9, 2023
20 of 24 checks passed
@tgodzik tgodzik deleted the update-scalameta branch October 9, 2023 12:43
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