-
Notifications
You must be signed in to change notification settings - Fork 44
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
Add highlight support for YAML #174
Conversation
cc985b4
to
60f5887
Compare
60f5887
to
8c728b8
Compare
private val key: CodeSpanParser = { | ||
CodeSpanParser((keyName ~ oneOf(':') ~ anyOf(' ','\t') ~ oneOf('\n')).map { r => | ||
List(r._1._1._1, CodeSpan(r._1._1._2 + r._1._2 + r._2)) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would need a similar refinement as the keyAndValue
parser for optional spaces before the colon.
val doubleQuotedStrLiteral = StringLiteral.singleLine('"') | ||
val singleQuotedStrLiteral = StringLiteral.singleLine('\'') | ||
val number = NumberLiteral.decimalFloat | NumberLiteral.decimalInt | ||
val boolean = (literal("true") | literal("false")).map(b => Seq(CodeSpan(b, CodeCategory.BooleanLiteral))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
YAML has a long list of valid boolean literals:
val boolean = Keywords(CodeCategory.BooleanLiteral)(
"true","True","TRUE","false","False","FALSE","on","On","ON","off","Off","OFF","yes","Yes","YES","no","No","NO"
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It depends on YAML spec version, in 1.2 it's only 2 literals: go-yaml/yaml#214
Anyway, I think I will go with 1.1 as you suggested, probably it's makes more sense to be more liberal for highlighting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't aware of that change, I looked at an unversioned grammar document on yaml.org and tested a few common parsers like pyyaml
which apparently never got updated for YAML 1.2.
I don't have a strong opinion either way, but if the 1.1 booleans are still in wide-spread use it might indeed make sense to support them.
private val valueLiteral = { | ||
val doubleQuotedStrLiteral = StringLiteral.singleLine('"') | ||
val singleQuotedStrLiteral = StringLiteral.singleLine('\'') | ||
val number = NumberLiteral.decimalFloat | NumberLiteral.decimalInt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Octal and hex integers are also supported (and quick to add here).
attrName("kind"), colon, string("'ConfigMap'"), nl, | ||
attrName("metadata"), other(":\n "), | ||
// It would be more accurate if the space of `other(" ")` was part of its preceding string("port_overriden") | ||
// but in the end it does not matter how space will be colored |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest to remove this comment as it is misleading. In fact, the way you implemented it is exactly correct for two reasons: a) comparable highlighters in Laika behave in the same way, b) a YAML parser would not include the whitespace before the #
in the result.
| - key: value | ||
|``` | ||
""".stripMargin | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to vary the syntax a bit with optional spaces before and after the colon to test the suggested changes.
There are a few more comments this time, but I think they are all fairly simple changes. I'll look at the final PR for Dhall in the coming days, but that might take a tad longer as it seems to be a slightly bigger piece of work and I don't know Dhall very well. |
(keyName ~ oneOf(':') ~ oneOf(' ','\t')).map { r => | ||
List(r._1._1, CodeSpan(r._1._2 + r._2)) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The actual syntax for key+value is much less restrictive. The precise grammar is zero or more spaces before the colon and one or more after it.
I'd suggest the following alternative (untested, but probably fine):
private val keyAndValue = {
val separator = (anyOf(' ','\t') ~ ":" ~ someOf(' ','\t')).source.map(s => List(CodeSpan(s)))
(keyName ~ separator ~ valueLiteral).concat
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, it worked perfectly
8c728b8
to
f4c2f33
Compare
f4c2f33
to
7e74078
Compare
@jenshalm I addressed your comments, it's ready for re-review |
No description provided.