Skip to content

Don't highlight builtins used as kwarg. #17

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

Closed

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Mar 4, 2017

Don't highlight builtins immediately followed by an equal sign, as they
are likely being used as keyword argument to a function, e.g.
func(min=foo, max=bar). (Similarly to the case of attributes named
using a builtin, it is unlikely that client code has the choice there
anyways.)

Don't highlight builtins immediately followed by an equal sign, as they
are likely being used as keyword argument to a function, e.g.
`func(min=foo, max=bar)`.  (Similarly to the case of attributes named
using a builtin, it is unlikely that client code has the choice there
anyways.)
@nfnty
Copy link
Member

nfnty commented Mar 4, 2017

I think this is counterproductive, as the kwarg is overriding a builtin function and therefore should be highlighted differently.

@anntzer
Copy link
Contributor Author

anntzer commented Mar 5, 2017

From the point of vue of client code (calling the function), there is no overriding happening (and, as mentioned above, client code doesn't really have a choice). Given that this is the most common case (you only define a function once, but it is (usually...) called multiple times), I think it should have priority.

At the function definition site, I agree that a different highlighting may be better. If one really wants to implement this, one could try to see whether you're in a function signature e.g. by checking if the next closing parenthesis is followed by a colon (modulo a return value annotation). However, in practice, the override is only a potential issue if the argument is used at least once (otherwise, it is harmless). In that case, the overridden builtin will at least appear highlighted as a builtin once in the body of the function.

@nfnty
Copy link
Member

nfnty commented Mar 5, 2017

Trying to parse for definition vs call is going to be very complex and not worth it.

This is obviously subjective and therefore should be user configurable and disabled by default.

@anntzer
Copy link
Contributor Author

anntzer commented Mar 5, 2017

Sounds OK to me. However it should probably be a disabling option, e.g. g:python_dont_highlight_builtin_as_kwarg. How would you like it to interact with highlight_all and highlight_builtins? (I don't have a strong preference but we may as well get it right from the beginning.)

@nfnty
Copy link
Member

nfnty commented Mar 5, 2017

No, the option should be called g:python_highlight_builtin_funcs_kwarg and be enabled by default, regardless of other settings and only be used when g:python_highlight_builtin_funcs is enabled.

@nfnty
Copy link
Member

nfnty commented Mar 5, 2017

I'll implement it.

@nfnty nfnty closed this in aacf27d Mar 5, 2017
@nfnty
Copy link
Member

nfnty commented Mar 5, 2017

Thanks!

@anntzer anntzer deleted the dont-highlight-keyword-args branch March 5, 2017 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants