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

[Perl] Combine variable.function with support.function #1841

Closed

Conversation

deathaxe
Copy link
Collaborator

@deathaxe deathaxe commented Jan 16, 2019

This commit follows an recent proposal from the Go.sublime-syntax to combine the general variable.function with the support.function scope in order to provide the option for dedicated highlighting of builtin functions via color-scheme.

As a side effect all builtin functions are made available via "Goto Reference".

This commit follows an recent proposal from the Go.sublime-syntax to
combine the general `variable.function` with the `support.function`
scope in order to provide the option for dedicated highlighting of
builtin functions via color-scheme.

As a side effect all builtin functions are made available via
"Goto Reference".
@deathaxe
Copy link
Collaborator Author

Realized support.function to be part of the indexed reference list, already.

Is this combination of variable.function and support.function desired or is it an accident in Go?

@FichteFoll
Copy link
Collaborator

FichteFoll commented Jan 19, 2019

I use variable.function support.function in Python as well, since you want to target variable.function when adding special highlighting of function usage, e.g. in function calls.

Exact scope is source.python meta.function-call.python meta.qualified-name.python variable.function.python support.function.magic.python for df.__init__().

Example:

filter(functools.partial(isinstance, str), range(10))

2019-01-19_16-11-54

variable.function are in italics, support.function are green, support.type blue.

  1. filter, partial, isinstance are functions, but only filter and partial are called while isinstance is used as a reference.
  2. filter and isinstance are built-in functions, partial is not (part of the stdlib)
  3. range is highlighted as a function because it is used like one, though its technically a class constructur.
  4. str is just a type constructor, though if it was invoked it'd be classified as a function invocation since (the) Python (syntax) does not differentiate between constructor calls for classes/built-in types and normal functions.

TL;DR: I believe there is merit in having the two scopes.

@keith-hall
Copy link
Collaborator

To me, scoping a built-in function as variable.function just to reinforce that it is being called, is not the best approach - either it is a user-defined variable function or a built-in support function...
Maybe it would be better to add a meta scope to the identifier when it is being called, like we have meta.function-call for the entire path and parameters, we could have meta.function-invocation for only the identifier itself.

This then honors the Scoping Naming guidelines, emphasis mine:

The name of the function or method should be variable.function, unless the function is scoped with support.function.

@FichteFoll
Copy link
Collaborator

Maybe it would be better to add a meta scope to the identifier when it is being called

Seems likely. What about meta.function-call.callee? Should that span the entire path or only the last identifier as variable.function does currently? The former seems to be the more semantically correct one and a selector on meta.function-call.callee & (support.function | variable.function) would match fine for my use case, assuming the function callee match doesn't expand to function calls inside like decorator(param)(func), but I'm not aware of any syntax doing that.

I don't think anyone besides me uses the scopes like this, so we don't need to worry about backwards compatibility.

@deathaxe
Copy link
Collaborator Author

The meta approach seems related with #884, which proposed meta.function.name.

Either way a scope to narrow down the function identifier would be useful as some functions may also allow macros being used as function name.

// define macro
#define  MACRO as func_name


// usage
MACRO(args)

What is that? A variable.function.macro, variable.function constant.other.macro, meta.function-call.name constant.other.macro?

About the range of the meta

Spanning the whole path would be most suitable, but may be difficult in some languages depending on complexity. May also have a significant impact on performance.

@deathaxe
Copy link
Collaborator Author

deathaxe commented Jan 22, 2019

I follow @keith-hall with not stacking variable.function and support.function and close this PR.

We might ask @mitranim change the Go syntax accordingly then.

@deathaxe deathaxe closed this Jan 22, 2019
@FichteFoll
Copy link
Collaborator

We should formulate our discoveries in this thread into an RFC-like issue for documentation.

@mitranim
Copy link
Contributor

mitranim commented Feb 2, 2019

I think I accidentally (mis)took variable.function to mean "function call" and support.function to mean "reference to known function", which are different roles. It never occurred to me that support.function could be used to indicate a function call, or that variable.function could be used for a non-function-call. My color schemes apply a "function call" color to both. I happen to mostly work in Go, which currently does not scope non-calls as support.function. This makes it easy to spot function calls on a glance and tell them apart from function references. I'm not sure if this can be applied to other languages, or if this general notion would make sense to others, but it's something to consider.

@mitranim
Copy link
Contributor

mitranim commented Feb 2, 2019

Oh yeah, quote from scope naming:

Function and method names should be scoped using the following, but only when they are being invoked.

variable.function

This particular section doesn't mention support.function. Makes it pretty clear that function calls should be variable.function. That's probably where I got the conviction.

@deathaxe deathaxe deleted the pr/perl-opt-support-funcs branch February 3, 2019 12:26
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.

4 participants