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

Improve highlighting of in. #169

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

mcol
Copy link

@mcol mcol commented Mar 11, 2022

This highlights both in keywords in multi-iterable for loops (#148), and adds highlighting in the cases where in is used to check if an element is contained in an array, as in if 2 in [1, 2, 3].

This highlights both `in` keywords in multi-iterable `for` loops, and adds
highlighting when `in` is used to check if an element is contained in an
array.
@tpapp
Copy link
Collaborator

tpapp commented Mar 11, 2022

Can you please add some tests, eg the cases mentioned in #148? You will find examples in the current test script with names like julia--test-...-font-lock.

@non-Jedi
Copy link
Contributor

in is functioning as just a normal infix function when not used in a for-loop and so shouldn't be font-locked as a keyword.

@mcol
Copy link
Author

mcol commented Mar 11, 2022

@non-Jedi I didn't see your comment before pushing my commit. As a newbie user, I think it would help to font-lock in also outside a loop, but I can revert that.

@non-Jedi
Copy link
Contributor

My most convincing objection is that in(x, y) has the exact same syntactical meaning as x in y (it even parses the same; try it yourself with Meta.parse). If we font-lock in for x in y, it doesn't make sense not to font-lock it for in(x, y), and if we're font-locking in(x, y), it's only going to look confusing next to the non-font-locked foo(x, y).

I kinda get it. Infix function symbols are parsed in a way that's different from normal symbols. isa is the same way. For consistency's sake, if we were going to font-lock in and isa differently we would also need to font-lock all in-fix operators (+, *, etc.).

@mcol
Copy link
Author

mcol commented Mar 12, 2022

Sure, the more I thought of that and played with edge cases, the more I agree with you. Clearly, that makes a fix much harder, as you mentioned in the issue. I still have a couple ideas I'd like to try, but it'll be a few days before I can get back to this.

@tpapp
Copy link
Collaborator

tpapp commented Mar 12, 2022

FWIW, I would prefer just font-locking in for. I realize it is not easy to do this.

@non-Jedi
Copy link
Contributor

@mcol, would you be interested in updating this pull request just to add the multi-loop in test cases and mark them as broken?

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.

3 participants