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

enh(swift) Improved highlighting for types and generic arguments (#2920) #2920

Merged
merged 3 commits into from Dec 17, 2020
Merged

enh(swift) Improved highlighting for types and generic arguments (#2920) #2920

merged 3 commits into from Dec 17, 2020

Conversation

svanimpe
Copy link
Contributor

Here's the next step for the Swift grammar rewrite:

  • New grammar for types to better highlight type identifiers, including those used as generic arguments, as well as any keywords used within generic arguments.
  • Includes rules to prevent punctuation used within types (<, >, ?, !, ..., and &) from being highlighted as operators.
  • Fixes (swift) does not recognize core Apple foundational libraries #2758

With these improvements, I should be able to write a new grammar for function declarations, to fix the remaining issues #2895 and #2857

@joshgoebel
Copy link
Member

Awesome, more PRs this size would be great. :)

@svanimpe
Copy link
Contributor Author

Any comments about the relevance? Should any be set to 0 (or to 1, in case I set them to 0)?

@joshgoebel
Copy link
Member

Any comments about the relevance? Should any be set to 0 (or to 1, in case I set them to 0)?

I didn't see any issues. We want relevance for the "Apple like" libraries (for parity with Obj-C)... but nothing else is worthy of relevance... they are mostly too common "!?" or things that we don't typically give relevance for (random identifiers).

CHANGES.md Outdated Show resolved Hide resolved
}
);
// Generic arguments
TYPE.contains.push({
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
TYPE.contains.push({
TYPE.contains.push(TEMPLATE_WHATEVER_THINGY)

Should we perhaps give this rule a name with a constant vs just inlining it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

@joshgoebel joshgoebel left a comment

Choose a reason for hiding this comment

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

Lets tweak the release notes slightly but otherwise this looks good... perhaps consider if we should have a constant for that one thing or not, but I'm ok either way.

@svanimpe
Copy link
Contributor Author

One thing I should note here is that the following will still result in false matches:

(var: Int, let: Double)

Here, var and let are highlighted as keywords, even though they're used as identifiers for tuple elements (which is allowed). This should rarely happen, but still...

I will look into this with my next PR, as this is very similar to function parameters, which also need improving.

@joshgoebel
Copy link
Member

Doesn't need to happen here, but I wonder if we should just expand (AV|CA|CF|CG|CI|CL|CM|CN|CT|MK|MP|MTK|MTL|NS|SCN|SK|UI|WK|XC) to be a full list of the full names of all the Apple frameworks then use that in both ObjC and Swift? Perhaps not necessarily if we're not having issues, but it would be slightly more precise... someone else with a codebase of AV* objects is going to be sorely disappointed when their code is always detected as ObjC or Swift.

@joshgoebel joshgoebel changed the title (swift) New grammar for types enh(swift) Improved highlighting for types and generic arguments (#2920) Dec 17, 2020
@joshgoebel joshgoebel merged commit 1fb979c into highlightjs:master Dec 17, 2020
@svanimpe
Copy link
Contributor Author

I wonder if we should just expand (AV|CA|CF|CG|CI|CL|CM|CN|CT|MK|MP|MTK|MTL|NS|SCN|SK|UI|WK|XC) to be a full list of the full names of all the Apple frameworks

I don't think such a list even exists, and if it did, it would change every year. Apple just reserves all two-letter prefixes for its own use, even though it now also uses some three-letter prefixes.

Personally, I don't think the current list even belongs here, because Swift doesn't use prefixes. You only see these prefixes when interacting with Objective-C frameworks. I've only included it here to avoid misdetection :)

@joshgoebel
Copy link
Member

joshgoebel commented Dec 18, 2020

because Swift doesn't use prefixes. You only see these prefixes when interacting with Objective-C frameworks. I've only included it here to avoid misdetection :)

Right, but some of these frameworks are super-common frameworks that a lot of swift code might interact with because they are platform libraries, no? When you reference them in Swift you'd do it by name, no? A lot of Apple frameworks are still Obj-C are they not?

@svanimpe
Copy link
Contributor Author

Yes, Swift development on Apple platform still involves talking to a lot of Objective-C frameworks. So the prefixes are still relevant in that aspect, but they should become less relevant over the coming years, as people switch to SwiftUI.

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.

(swift) does not recognize core Apple foundational libraries
2 participants