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

Optional parenthesis for @command and @option #127

Closed
kdeldycke opened this issue Sep 21, 2022 · 5 comments
Closed

Optional parenthesis for @command and @option #127

kdeldycke opened this issue Sep 21, 2022 · 5 comments
Labels
enhancement New feature or request wontfix This will not be worked on

Comments

@kdeldycke
Copy link
Contributor

Since Click 8.1.0, we can use command and group decorators without parenthesis. See:

Nothing important here but a nice little nitpick. Not certain if it is worth considering for integration in cloup.

@kdeldycke kdeldycke added the enhancement New feature or request label Sep 21, 2022
kdeldycke added a commit to kdeldycke/click-extra that referenced this issue Sep 21, 2022
@janluke
Copy link
Owner

janluke commented Sep 21, 2022

I know they added it some time ago but I don't like this pattern:

  • it complicates the code and the type annotations for no practical benefit
  • I think it manages to confuse both beginners (making decorators appear more magical than they actually are) and more intermediate programmers (who wonder wtf is going on :)).

I know some popular libraries like pytest use it (but this is probably the least magical thing about pytest), but I'd gladly avoid it.

A good reason to add it is to be consistent with Click. And that's a pretty good reason. Still, I'd rather provide a good error message as the original issue and PR proposed. This is somewhat needed now that the syntax is allowed in Click.

@janluke
Copy link
Owner

janluke commented Sep 30, 2022

I might change my mind on this, given that even the standard library embraces this pattern (e.g. @dataclass). I still think the way it complicates the code (and the way it complicates the semantics of the decorator giving it a sort of "double nature" (*)) is absolutely not worthwhile.

(*) it's basically overloading with radically different return types, something similar to Union return types, which I consider code smell.

@kdeldycke
Copy link
Contributor Author

kdeldycke commented Sep 30, 2022

Oh yes. I agree with you sentiment. Implementing these naked decorators is a chore, even if it makes them beautiful from the user point of view. That's why I classify them as nice-to-have, not critical. I don't even have a clue on how to implement them! 😅

Good point on @dataclass. If they're part of the standard library maybe will see some stuff in the future to make these easier to implement. In the mean I guess we'll have to sacrifice purity for convenience and user-friendliness.

@janluke
Copy link
Owner

janluke commented Oct 2, 2022

When you don't care about static typing and IDE autocompletion, it's pretty easy to implement. One can write a write a generic decorator, but you'll lose auto-completion:

from functools import wraps
from typing import Any, Callable, TypeVar

AnyCallable = Callable[..., Any]

F = TypeVar('F', bound=AnyCallable)
"""Type variable for a Callable."""

Decorator = Callable[[F], F]
DecoratorFactory = Callable[..., Decorator[F]]


def allow_missing_parenthesis(dec_factory):
    @wraps(dec_factory)
    def new_factory(*args, **kwargs):
        if args and callable(args[0]):
            return dec_factory(*args[1:], **kwargs)(args[0])
        return dec_factory(*args, **kwargs)

    return new_factory


#
# Usage example
#


@allow_missing_parenthesis
def decorator_factory(name="<default name>") -> Decorator[F]:
    """A dummy decorator factory."""

    def decorator(f: F) -> F:
        @wraps(f)
        def wrapper(*args, **kwargs):
            f(*args, **kwargs)
            print(f"I'm {name}.")
            print("---")

        return wrapper

    return decorator


@decorator_factory
def without_parenthesis(msg="[@decorator_factory]"):
    print(msg)


@decorator_factory()
def empty_args(msg="[@decorator_factory()]"):
    print(msg)


@decorator_factory("janluke")
def with_args(msg='[@decorator_factory("janluke")]'):
    print(msg)


without_parenthesis()
empty_args()
with_args()

Integrating the feature into Cloup and make all mypy type checks pass is more complicated. I won't do that work for such a cosmetic feature (which I don't even want or like).

@janluke janluke added the wontfix This will not be worked on label Jan 24, 2023
@janluke janluke closed this as completed Feb 25, 2023
@kdeldycke
Copy link
Contributor Author

Thanks @janluke for the code example! I'm quite bad at decorators so it took me a while to digest and integrate. I took the liberty to reuse some of your proposed code to add to click_extra at: https://github.com/kdeldycke/click-extra/blob/bceac30592e67f262a87af1e8cdbcf4bcac0e15e/click_extra/decorators.py#L31-L76 .

This helped me to clone @command and @group decorators from cloup while allowing their usage without parenthesis.

Thanks again @janluke ! 🤗

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants