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

(swift) Improved highlighting for functions, initializers, and subscripts #2930

Merged
merged 6 commits into from
Dec 28, 2020
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 28 additions & 34 deletions src/languages/swift.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ import {

/** @type LanguageFn */
export default function(hljs) {
const WHITESPACE = {
begin: /\s+/,
relevance: 0
};
// https://docs.swift.org/swift-book/ReferenceManual/LexicalStructure.html#ID411
const BLOCK_COMMENT = hljs.COMMENT(
'/\\*',
Expand Down Expand Up @@ -215,22 +219,22 @@ export default function(hljs) {

// https://docs.swift.org/swift-book/ReferenceManual/Attributes.html
const AVAILABLE_ATTRIBUTE = {
begin: /(@|#)available\(/,
end: /\)/,
keywords: {
$pattern: /[@#]?\w+/,
keyword: Swift.availabilityKeywords
.concat([
"@available",
"#available"
])
.join(' ')
},
contains: [
...OPERATORS,
NUMBER,
STRING
]
begin: /(@|#)available/,
className: "keyword",
starts: {
contains: [
{
begin: /\(/,
end: /\)/,
keywords: Swift.availabilityKeywords.join(' '),
contains: [
...OPERATORS,
NUMBER,
STRING
]
}
]
}
joshgoebel marked this conversation as resolved.
Show resolved Hide resolved
};
const KEYWORD_ATTRIBUTE = {
className: 'keyword',
Expand Down Expand Up @@ -291,8 +295,7 @@ export default function(hljs) {
// https://docs.swift.org/swift-book/ReferenceManual/Expressions.html#ID552
// Prevents element names from being highlighted as keywords.
const TUPLE_ELEMENT_NAME = {
begin: lookahead(concat(Swift.identifier, /\s*:/)),
end: lookahead(/:/),
begin: concat(Swift.identifier, /\s*:/),
Copy link
Member

Choose a reason for hiding this comment

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

I didn't understand why we needed lookahead at all here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is probably a leftover from when I used contains. The goal here was to check that the upcoming code is indeed an identifier, but leave matching to the contained rules.

Will the keywords: "_|0" rule still work with this change?

Copy link
Member

@joshgoebel joshgoebel Dec 26, 2020

Choose a reason for hiding this comment

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

Yes, keywords match the buffer, they do not consume the input.

  • begin matches
  • end matches
  • buffer = [begin, text in middle, end].concat()

That whole buffer is fed into the keyword engine for processing. Of course children rules that match can flush the buffer... but it's easy enough to reason about in a simple case with no children.

That's why keywords can get difficult in edge cases, because they are happening absolutely last and at the mercy of the whole ruleset.

keywords: "_|0",
relevance: 0
};
Expand Down Expand Up @@ -321,21 +324,18 @@ export default function(hljs) {
// Matches both the keyword func and the function title.
// Grouping these lets us differentiate between the operator function <
// and the start of the generic parameter clause (also <).
const FUNCTION_TITLE = {
const FUNC_PLUS_TITLE = {
beginKeywords: 'func',
contains: [
{
begin: /\s+/,
relevance: 0
},
{
className: 'title',
begin: either(QUOTED_IDENTIFIER.begin, Swift.identifier, Swift.operator),
// Required to make sure the opening < of the generic parameter clause
// isn't parsed as a second title.
endsParent: true,
relevance: 0
}
},
WHITESPACE
]
};
const GENERIC_PARAMETERS = {
Expand All @@ -351,7 +351,7 @@ export default function(hljs) {
lookahead(concat(Swift.identifier, /\s*:/)),
lookahead(concat(Swift.identifier, /\s+/, Swift.identifier, /\s*:/))
),
end: lookahead(/:/),
end: /:/,
Copy link
Member

Choose a reason for hiding this comment

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

No good reason not to consume this that I can see, so we just do that, since it's simpler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that should be fine. I didn't consume it just it case I need it to detect a type annotation (which always starts with :), but I managed to get proper highlighting without having to do that.

Copy link
Member

Choose a reason for hiding this comment

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

I didn't consume it just it case I need it to detect a type annotation

We really need look-behind. :( Lots of complexity dealing with these kind of things without it.

relevance: 0,
contains: [
{
Expand Down Expand Up @@ -386,13 +386,10 @@ export default function(hljs) {
className: 'function',
begin: lookahead(/\bfunc\b/),
contains: [
FUNCTION_TITLE,
FUNC_PLUS_TITLE,
GENERIC_PARAMETERS,
FUNCTION_PARAMETERS,
{
begin: /\s+/,
relevance: 0
}
WHITESPACE
],
illegal: /\[|%/
};
Expand All @@ -409,10 +406,7 @@ export default function(hljs) {
contains: [
GENERIC_PARAMETERS,
FUNCTION_PARAMETERS,
{
begin: /\s+/,
relevance: 0
}
WHITESPACE
],
illegal: /\[|%/
};
Expand Down