Fix #3814 Correct highlighting issues in REPL #3987
Conversation
| import scala.io.Source | ||
|
|
||
| /** Runs all tests contained in `compiler/test-resources/printing/` | ||
| * To check test cases, you can use "cat" or "less -r" from bash*/ |
There was a problem hiding this comment.
Feature request: add a mode to generate the test files from the syntax highlighter output, this would be useful since you cannot just copy-paste the highlighted output when the test files need to be updated.
There was a problem hiding this comment.
What do you think about this?
private def testFile(f: JFile): Unit = {
val linesIt = Source.fromFile(f).getLines()
val input = linesIt.takeWhile(_ != "result:").mkString("\n")
val expectedOutput = linesIt.mkString("\n")
val actualOutput = SyntaxHighlighting(input).mkString
if (expectedOutput != actualOutput) {
println("Expected output:")
println(expectedOutput)
println("Actual output:")
println(actualOutput)
if (f.getName == "nameOfFileToConvertToATest") generateTestFile()
fail(s"Error in file $f, expected output did not match actual")
}
def generateTestFile(): Unit = {
val path = "compiler/test-resources/printing/" + f.getName
new PrintWriter(path) {
write(input + "\nresult:\n" + actualOutput)
close()
}
println(s"Test file for ${f.getName} has been generated")
}
}Originally I used a simple function which wrote to a file in the project's root folder and copied its content manually after "result:" line.
This generateTestFile function more convenient. However I'm not sure if it needs explaining and if yes how much.
(it works on a plain scala file and on a changed test file as well)
benkobalog
force-pushed
the
fix-#3814-2
branch
from
aaeff51 to
194d5d5
Compare
|
@smarter Can please take a look? |
|
Not available this week, maybe @allanrenucci can have a look? |
| case _ => false | ||
| } | ||
|
|
||
| def shouldUpdateLastToken(currentPotentialToken: String): Boolean = |
There was a problem hiding this comment.
Could you add a documentation comment for this method?
There was a problem hiding this comment.
Yes, I will write documentation to this.
Also I'm thinking about renaming currentPotentialToken to just currentToken. "Potential" wanted to express that it might be the next "lastToken", but now I don't think it adds any value to the name.
Correct highlighting after "|", ":", "&" characters and after "case", "val", "val" when using extractors.
Added comments to updateLastToken function Use syntax highlight tests proposed in pr scala#4441 instead of the file based solution
|
@smarter I added some documentation, but I also refactored the function a bit. |
| case _ => false | ||
| val valDefStarterTokens = "var" :: "val" :: "def" :: "case" :: Nil | ||
|
|
||
| /** lastToken is used to check whether we want to show something |
There was a problem hiding this comment.
Maybe we shouldn't call it lastToken then since it's not the last token anymore :).
| } | ||
|
|
||
| @Test | ||
| def valVarDef = { |
There was a problem hiding this comment.
Could you split the tests below into smaller tests, that that test only one specific thing. E.g:
@Test
def unionTypes = {
test("type A = String|Int| Long", "<K|type> <T|A> = <T|String>|<T|Int>| <T|Long>")
test("type B = String |Int| Long", "<K|type> <T|B> = <T|String> |<T|Int>| <T|Long>")
test("type C = String | Int | Long", "<K|type> <T|C> = <T|String> | <T|Int> | <T|Long>")
test("type D = String&Int& Long", "<K|type> <T|D> = <T|String>&<T|Int>& <T|Long>")
test("type E = String &Int& Long", "<K|type> <T|E> = <T|String> &<T|Int>& <T|Long>")
test("type F = String & Int & Long", "<K|type> <T|F> = <T|String> & <T|Int> & <T|Long>")
test("fn[String|Char](input)", "fn[<T|String>|<T|Char>](input)")
}There was a problem hiding this comment.
Also note that the code snippets don't need to compile. So if for example you want to test syntax highlighting for pattern matching, you don't need to define an ADT for it
|
@allanrenucci @smarter Thanks for the feedback! I changed both things you said. |
|
Thanks @benkobalog |
Correct highlighting after "|", ":", "&" characters
and after "case", "val", "val" when using extractors.
I added test cases to cover the faulty cases in the ticket, plus some basic cases