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) highlight function and macro call usage #3959

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

bradleymackey
Copy link
Contributor

@bradleymackey bradleymackey commented Jan 7, 2024

Improve fidelity by highlighting function calls in addition to definitions. This is already done by javascript, for example.

Before After
Screenshot 2024-01-07 at 14 03 24 Screenshot 2024-01-07 at 14 05 49

Changes

  • Add negativeLookahead helper to lib/regex
  • Highlight all function and method calls as title.function (e.g. myFunction(), obj.myFunction()).
  • Highlight all freestanding macro calls as title.function (e.g. #myMacro()).
  • There can be >=0 whitespace between the end of the function name and the parentheses, but newlines are not permitted. Encapsulate this in a new regex, TRAILING_PAREN_REGEX
  • Built-ins still highlight as such but with exceptions for methods–where they are semantically no longer builtins.
  • Most keywords can exist as both properties and methods (e.g. obj.if()). Highlight keyword method calls also.
    • Number sign keywords and attribute keywords are permitted as raw functions, as they start with leading # and @ respectively. If they don't start with these, they're no longer considered reserved.
  • Highlight escaped function name calls also (e.g. `myFunc`()). This feature exists so that reserved words can be used as identifiers.
  • Variable accesses (e.g. obj.myVariable) remain unhighlighted for now.
  • Add markup tests for all the cases and edge cases described.

Next Steps (for a future PR)

  • Support trailing closures, which are method calls: exampleCall { print("action") }

Checklist

  • Added markup tests
  • Updated the changelog at CHANGES.md

Copy link

github-actions bot commented Jan 7, 2024

Build Size Report

Changes to minified artifacts in /build, after gzip compression.

4 files changed

Total change +241 B

View Changes
file base pr diff
es/core.min.js 8.17 KB 8.17 KB +1 B
es/highlight.min.js 8.17 KB 8.17 KB +1 B
es/languages/swift.min.js 3.55 KB 3.67 KB +120 B
languages/swift.min.js 3.56 KB 3.68 KB +119 B

Copy link

github-actions bot commented Jan 7, 2024

Build Size Report

Changes to minified artifacts in /build, after gzip compression.

3 files changed

Total change +231 B

View Changes
file base pr diff
es/languages/swift.min.js 3.55 KB 3.67 KB +116 B
highlight.min.js 8.21 KB 8.21 KB -1 B
languages/swift.min.js 3.56 KB 3.68 KB +116 B

Copy link

github-actions bot commented Jan 7, 2024

Build Size Report

Changes to minified artifacts in /build, after gzip compression.

5 files changed

Total change +241 B

View Changes
file base pr diff
es/core.min.js 8.17 KB 8.17 KB +1 B
es/highlight.min.js 8.17 KB 8.17 KB +1 B
es/languages/swift.min.js 3.55 KB 3.67 KB +120 B
highlight.min.js 8.21 KB 8.21 KB -1 B
languages/swift.min.js 3.56 KB 3.68 KB +120 B

Copy link

github-actions bot commented Jan 7, 2024

Build Size Report

Changes to minified artifacts in /build, after gzip compression.

5 files changed

Total change +207 B

View Changes
file base pr diff
es/core.min.js 8.17 KB 8.18 KB +2 B
es/highlight.min.js 8.17 KB 8.18 KB +2 B
es/languages/swift.min.js 3.55 KB 3.65 KB +101 B
highlight.min.js 8.21 KB 8.21 KB +2 B
languages/swift.min.js 3.56 KB 3.66 KB +100 B

@joshgoebel
Copy link
Member

We can't use negative lookahead until v12 though because it's a breaking change. And I don't have a timetable on that yet.

@bradleymackey
Copy link
Contributor Author

bradleymackey commented Jan 10, 2024

@joshgoebel This is an adapted version of Javascript's implementation which is already using negative lookahead ( see here). My understanding was that only lookbehind is not allowed currently.

Copy link

Build Size Report

Changes to minified artifacts in /build, after gzip compression.

5 files changed

Total change +207 B

View Changes
file base pr diff
es/core.min.js 8.17 KB 8.17 KB +2 B
es/highlight.min.js 8.17 KB 8.17 KB +2 B
es/languages/swift.min.js 3.55 KB 3.65 KB +101 B
highlight.min.js 8.2 KB 8.21 KB +2 B
languages/swift.min.js 3.56 KB 3.66 KB +100 B

@bradleymackey
Copy link
Contributor Author

@joshgoebel Is there a way forward for this PR? If the concern is a language-specific breaking change, the Swift language itself is already using negative lookahead as well, here.

@joshgoebel
Copy link
Member

joshgoebel commented Jul 6, 2024

Sorry for delay. I think title.function might not be correct though typically we consider anything after . hanging off of an identifier (likely an object) a property... if the function is "global" or standalone then it could get function.title. IE:

debug_it("hello world")  # debug_it is `title.function` (seems to be global function call)
person.eat("sandwich") # `eat` is a property of person, that also appears to be a method/function

Thoughts?

const KEYWORD_GUARD = {
// Consume .keyword to prevent highlighting properties and methods as keywords.
match: concat(/\./, either(...Swift.keywords)),
const KEYWORD_PROP_GUARD = {
Copy link
Member

Choose a reason for hiding this comment

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

Ok, so we don't want the keyword engine to grab these as keywords, true, but if they are essentially "properly access" why wouldnt we scope them as that?

{
// Functions and macro calls
scope: "title.function",
keywords: KEYWORDS,
Copy link
Member

Choose a reason for hiding this comment

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

This seems weird, why would we have keywords popping up in the middle of a function name? Or is this truly to cover "it's a function name that's also a keyword"?

TRAILING_PAREN_REGEX,
],
scope: {
2: "title.function",
Copy link
Member

Choose a reason for hiding this comment

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

We'd call anything that appears to be an object property access a property for consistent highlighting across different grammars.

* @param {RegExp | string } re
* @returns {string}
*/
export function negativeLookahead(re) {
Copy link
Member

Choose a reason for hiding this comment

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

Awesome.

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.

2 participants