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) Add support for closure arguments (#2871) #2907

Closed
wants to merge 4 commits into from
Closed

enh(swift) Add support for closure arguments (#2871) #2907

wants to merge 4 commits into from

Conversation

il3ven
Copy link
Contributor

@il3ven il3ven commented Dec 4, 2020

Resolves #2871

Changes

  1. Added a new mode to handle closure arguments such as $0 or $1
  2. This new mode has also been added as a submode inside STRING because of the following reason

Checklist

  • Added markup tests, or they don't apply here because...
  • Updated the changelog at CHANGES.md
  • Added myself to AUTHORS.txt, under Contributors

@svanimpe
Copy link
Contributor

svanimpe commented Dec 4, 2020

Hi @il3ven

I kinda fixed this already 😅 I'm trying to redo the entire grammar and am working on a PR that will include a complete grammar for identifiers, which will handle not only closure arguments, but also property wrapper projections.

@il3ven
Copy link
Contributor Author

il3ven commented Dec 4, 2020

@svanimpe That's great if you have fixed it. 😅

I saw your PR and it mentioned issues #2819 and #2820 so that is why I picked this.

Anyway, can I assume that all the other issues related to swift that was opened by you will be covered in your PR?

@svanimpe
Copy link
Contributor

svanimpe commented Dec 4, 2020

@il3ven I hope to tackle most of them yes 🙂 But some are pretty tricky. For example, to fix #2895 I will have to be able to fully parse function declarations, so I can detect a complete declaration, excluding the body (as there is no body in a protocol). But that means first parsing types, identifiers, attributes, operators, etc, which I what I'm working on now.

@svanimpe
Copy link
Contributor

svanimpe commented Dec 5, 2020

@il3ven @joshgoebel An interesting question here is: Do we want to highlight implicit paramters ($0, $1, etc) as variables? According to the grammar, they're just identifiers, so should we highlight them separately? If so, should we also highlight normal identifiers, or just the ones that start with a dollar sign?

@il3ven
Copy link
Contributor Author

il3ven commented Dec 5, 2020

@svanimpe I chose to highlight implicit parameters ($0, $1, etc) as variables because in code they are used as variables and it made the most sense visually.

I don't know much about swift and I am getting started with open source. But I think we should only highlight the ones that start with a dollar sign or else we will end up highlighting a lot of words.

@svanimpe
Copy link
Contributor

svanimpe commented Dec 5, 2020

Xcode does highlight a lot of identifiers, but that highlighting is based on information from the compiler, which can tell the difference between different types of variables. That's not possible with a regex highlighter, so I'd also opt for only highlighting 'special' identifiers like this.

Here's the grammar I have ready to go for an upcoming PR (but I think I'm flooding @joshgoebel a bit now 😅):

  // https://docs.swift.org/swift-book/ReferenceManual/LexicalStructure.html#ID412
  const identifierHead = either(
    /[a-zA-Z_]/,
    /[\u00A8\u00AA\u00AD\u00AF\u00B2–\u00B5\u00B7–\u00BA]/,
    /[\u00BC–\u00BE\u00C0–\u00D6\u00D8–\u00F6\u00F8–\u00FF]/,
    /[\u0100–\u02FF\u0370–\u167F\u1681–\u180D\u180F–\u1DBF]/,
    /[\u1E00–\u1FFF]/,
    /[\u200B–\u200D\u202A–\u202E\u203F–\u2040\u2054\u2060–\u206F]/,
    /[\u2070–\u20CF\u2100–\u218F\u2460–\u24FF\u2776–\u2793]/,
    /[\u2C00–\u2DFF\u2E80–\u2FFF]/,
    /[\u3004–\u3007\u3021–\u302F\u3031–\u303F\u3040–\uD7FF]/,
    /[\uF900–\uFD3D\uFD40–\uFDCF\uFDF0–\uFE1F\uFE30–\uFE44]/,
    /[\uFE47–\uFFFD]/
  );
  const identifierCharacter = either(
    identifierHead,
    /\d/,
    /[\u0300–\u036F\u1DC0–\u1DFF\u20D0–\u20FF\uFE20–\uFE2F]/
  );
  const IDENTIFIER = {
    keywords: SWIFT_KEYWORDS,
    begin: `${identifierHead}${identifierCharacter}*`
  };
  const QUOTED_IDENTIFIER = {
    begin: `\`${IDENTIFIER.begin}\``
  };
  const IMPLICIT_PARAMETER = {
    className: 'variable',
    begin: /\$\d+/
  };
  const PROPERTY_WRAPPER_PROJECTION = {
    className: 'variable',
    begin: `\\$${identifierCharacter}+`
  };

@joshgoebel
Copy link
Member

Do we want to highlight implicit parameters ($0, $1, etc) as variables? According to the grammar, they're just identifiers, so should we highlight them separately?

That was my first instinct... we don't [generally] tend to highlight "just an identifier"... it needs to have more semantic context... variable, title, function name, built-in, etc... If you look at the higher fidelity #2500 issue it's possible that more categories could be added to extend nuance in some cases, but we'd have to agree on exactly what those things were in order for their support to really be meaningful.

@il3ven il3ven closed this Dec 5, 2020
@svanimpe
Copy link
Contributor

svanimpe commented Dec 5, 2020

@joshgoebel In my upcoming PR, I'll only highlight $identifiers as variables. Highlighting other identifiers may be tricky, because I've noticed that keywords can stop working because they match the same regexs as identifiers.
Or maybe I don't fully understand how keywords are matched yet.

@joshgoebel
Copy link
Member

MODEs are matched first and then keywords are found inside the "buffer" of content that a mode includes. So modes (done wrong) can definitely break keywords.

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) Shorthand parameter names are highlighted as numbers
3 participants