Skip to content

StdLexical now emits one ErrorToken for unclosed comments. #403

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
Jul 8, 2021

Conversation

martingd
Copy link
Contributor

Fixes issue #399

@SethTisue
Copy link
Member

(let us know if you need help understanding & addressing the CI failures)

@martingd
Copy link
Contributor Author

@SethTisue Thank you. I have fixed the ListBuffer issue (which was not about Scala.js but Scala 2.12 and below). The binary compatibility issue, I have tried to fix changing a new method from protected to private which is also correct because overriding it or calling it from a derived class does not make much sense. If it does not work, I will ask for help. :)

@martingd
Copy link
Contributor Author

martingd commented Jul 2, 2021

@SethTisue Please let me know if there is anything that prevents this and my other issue from being merged – e.g., if you would like something done in a different way, or simply don't like the changes.

@martingd
Copy link
Contributor Author

martingd commented Jul 5, 2021

@Philippus Also, rebased this branch on top of issue-397 as it depends on the test file added in issue-397.

@SethTisue SethTisue requested a review from Philippus July 8, 2021 01:39
import scala.util.parsing.input.Reader

import scala.collection.mutable.ListBuffer
import java.awt.RenderingHints.Key
Copy link
Member

Choose a reason for hiding this comment

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

I think this import isn't used.

Copy link
Contributor Author

@martingd martingd Jul 8, 2021

Choose a reason for hiding this comment

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

@Philippus you are absolutely correct. Sorry about that. The import actually made it's way into this file in the previous pull request (PR402). This PR only makes a few changes to this file.

  @Test
  def parseUnclosedComments: Unit = {
    object Lexer extends StdLexical
    import Lexer._

    assertEquals(
      List(Identifier("id"), ErrorToken("unclosed comment")),
      lex(Lexer, "id /*")
    )

    assertEquals(
      List(Identifier("id"), ErrorToken("unclosed comment")),
      lex(Lexer, "id /* ")
    )
  }

However, because branch issue-399 currently is on top of issue-397 you see the diff of both issues in this PR. I will remove the unused import and rebase this PR on top of 2.x.

The remaining input is consumed so the ErrorToken is the last token
emitted by the scanner.
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