jazzband / Watson

:watch: A wonderful CLI to track your time!
http://tailordev.github.io/Watson/
MIT License
2.46k stars 240 forks source link

Update click to 8 #432

Closed DavidOliver closed 2 years ago

DavidOliver commented 3 years ago

Warning: I don't know what I'm doing; I don't usually use Python and this is my first time with Watson's code, so this pull request should be treated with caution. But hopefully it can at least act as the start of a fix for use of click v8.

The cause of #430 seems to be this (click v8 changes):

TypeError is raised when parameter with multiple=True or nargs > 1 has non-iterable default. #1749

'Multiple options' at click documentation.

I remove multiple=True from several click options, which I don't believe are intended to be specified at the command line more than once as they seem to be flags rather than options which take (multiple) values.

I believe I've successfully run Watson's tests, which all seem to be passing.

By the way, will a new release of Watson be prepared soon, either with the temporary fix of specifying v7 of click or this fix? I ask as I'm currently working on my system config. :)

jmaupetit commented 3 years ago

Wow this is huge. Thanks :pray: We'll look into this soon.

mizhozan commented 3 years ago

I'm not pro Python user and don't much about test suits, but my test with this pr on my local computer works fine plus I did a simple pytest and there was 250 passed, 14 warning, 91 errors as following gist.

I simply did a gh pr checkout 432 and then pip install and check watson which works just fine. For the test I just did pytest from the command line, if it's supposed (didn't see the documentation on test).

DavidOliver commented 3 years ago

@mizhozan, I also had those fixture-related failures. The solution seemed to be install the dev requirements from requirements-dev.txt. (See the varying mock versions depending on the version of setuptools you have.)

mizhozan commented 3 years ago

@DavidOliver Thanks a lot, now the test runs perfectly with 341 passes and 14 warnings according to this gist.

@mizhozan, I also had those fixture-related failures. The solution seemed to be install the dev requirements from requirements-dev.txt. (See the varying mock versions depending on the version of setuptools you have.)

DavidOliver commented 3 years ago

@mizhozan Thanks. I think I managed to confuse myself regarding the versions of packages I had installed. I've just (re-?)re-installed click (8.0.1), and now I also have the deprection warnings when running the tests.

@jmaupetit Can you hold off on merging, please? I'll see if I can get rid of the deprecation warnings.

DavidOliver commented 3 years ago

The deprecation warnings in the tests are now gone (see last commit), but I'm out of my depth here and can't know whether or not more needs to be done to properly update to click 8.

From the changelog:

Redesign the shell completion system. #1484, #1622

Support Bash >= 4.4, Zsh, and Fish, with the ability for extensions to add support for other shells.

Allow commands, groups, parameters, and types to override their completions suggestions.

Groups complete the names commands were registered with, which can differ from the name they were created with.

The autocompletion parameter for options and arguments is renamed to shell_complete. The function must take ctx, param, incomplete, must do matching rather than return all values, and must return a list of strings or a list of CompletionItem. The old name and behavior is deprecated and will be removed in 8.1.

The env var values used to start completion have changed order. The shell now comes first, such as {shell}source rather than source{shell}, and is always required.

Regarding shell_complete, it looks like the args taken may not need updating, but I don't know about "matching rather than return all values" and the return values.

Regarding env var values to start completion, I've searched for source_ in Watson's code and found source_zsh in create-completion-script.sh. I think this is where I bow out and leave things for others with more knowledge and understanding of Watson and Python. Thanks!

MinchinWeb commented 3 years ago

Ran into this issue today. Would love to see this merged in.

Xiretza commented 2 years ago

It's been almost a year, what's the status on this?

jmaupetit commented 2 years ago

Rebased and merged in #471