Skip to content

Conversation

@msanders
Copy link
Contributor

@msanders msanders commented May 15, 2018

This brings swift-mode in line with other editors such as TextMate and Vim, which distinguish between function/method calls, properties, and enum declarations with syntax highlighting. Here's a before & after screenshot.

I'm pretty new to Elisp, so let me know if anything could be improved.

@msanders msanders force-pushed the ms/additional-highlighting branch from d5cd82c to 6c6e6a3 Compare May 15, 2018 20:10
@taku0
Copy link
Member

taku0 commented May 15, 2018

Thank you for the PR. I will review this on the weekends.

@taku0
Copy link
Member

taku0 commented May 19, 2018

Highlighting property accesses and method calls will be helpful.

The regexp for enum cases seems to be bit buggy and may confuse users (see https://gist.github.com/taku0/5ce67840ce2824409877a6be6988cd5f). Fixing it would be difficult, so I suggest removing it. Vim doesn't seem to highlight enum cases.

Another concern is method/function calls with blocks. The following is a method call,

foo.bar { } 

while the following is not

if foo.bar { } 

Options:

  • A. highlighting all method/function calls like property accesses
  • B. highlighting method/function calls without parenthesis as property accesses (this PR)
  • C. highlighting property accesses before curly braces as method/function calls

What's your opinion?

@msanders
Copy link
Contributor Author

msanders commented May 19, 2018

Good catch, I've removed the enum regex for now. The pattern used in TextMate is a bit complicated (involving nested regexes), so it will probably be rather involved to get it working. Although it may be possible using a function similar to the property call matcher.

Regarding trailing closures, I'm personally fine with option (B) as it matches the behavior in both TextMate and Vim, and other editors using TextMate's bundle. The drawback to including curly braces is it also highlights patterns such as if let foo = bar {} which is less than ideal. A workaround would be introducing lookahead for if statements (and potentially others I've missed), but that would be pretty involved and potentially prone to bugs. I think this gets us most of the way there while making things a lot more readable.

@msanders msanders changed the title Add support for highlighting function calls / properties / enum cases Add support for highlighting function calls / properties May 19, 2018
@taku0
Copy link
Member

taku0 commented May 20, 2018

It's reasonable. Could you squash the commits, rebase the branch onto #145, and fix conflicts? I will merge this after merging #145. Thank you!

* Expand regexes for built-ins * Update existing keywords to use regexp-opt for consistency * Move keyword comments to docstrings * Add compactMap * Add Foundation types * Address PR feedback * Simplify property matching
@msanders msanders force-pushed the ms/additional-highlighting branch from 95b4416 to be9709d Compare May 20, 2018 14:48
@msanders msanders force-pushed the ms/additional-highlighting branch from be9709d to f6b8b90 Compare May 23, 2018 03:01
@taku0
Copy link
Member

taku0 commented May 26, 2018

Merged abf3426

@taku0 taku0 closed this May 26, 2018
@msanders msanders deleted the ms/additional-highlighting branch May 26, 2018 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants