Port CLI to click

[untitaker]

• Color codes need to be ported, should retain compatibility with config values
• Setting context object inside function doesn't pass the new object to subcommands pallets/click#200 -- fixed in version 3.1
• Ability to invoke a subcommand with defaults pallets/click#201 -- fixed in 3.2
• ctx.invoked_subcommand is always None pallets/click#205 -- fixed in 3.2
• Write tests

Notes:

• Color codes are largely compatible, but they have different escape sequences.
• We depend on click 3.2

[geier]

Nice work! (I hope, am on holidays and while I had a look at the commits, I didn't actually run it).

I'd like to merge this first into branch configobj (which you are happy to do if you want 😈 ), then fix up the docs and finally merge configobj into master and do a 0.3 release.

My reasoning is this:

a lot changed (default behaviour, config file format and dependencies) and I'd like to avoid the state where the online documentaion is either out of sync with current github master or released version/packages. Once the complete doc is in sphinx/on rtd the handy version number
switcher will hopefully avoid some confusion.

Also: before you put more time into refactoring config parsing, have a look at branch configobj (most of the old stuff will leave us).

Regarding the invocation of khal:
Perhaps this really isn't so bad, me (and everyone else who wants this) could just add an alias to their shell. This should make writing properly working completion files much easier :) .

How does the current version behave if a -a CAL and a -d CAL option are given?

[untitaker]

Also: before you put more time into refactoring config parsing, have a
look at branch configobj (most of the old stuff will leave us).

I wasn't sure whether configobj is even maintained anymore, found the port to Python 3 only recently (Michael Foord's site seems to be unmaintained). I assume you're using this variant? https://github.com/DiffSK/configobj/blob/master/README.md

I'll have a look at the branch. What's the progress on that one?

Regarding the invocation of khal:
Perhaps this really isn't so bad, me (and everyone else who wants this)
could just add an alias to their shell. This should make writing
properly working completion files much easier :) .

Glad to hear that, the hackery to make this work would be really bad.

How does the current version behave if a -a CAL and a -d CAL option
are given?

The validation happens with custom code. click.UsageError is raised inside the function body, which then looks like a normal error message in the console.

[geier]

Quoting Markus Unterwaditzer (2014-08-13 23:23:09)

Also: before you put more time into refactoring config parsing, have a
look at branch configobj (most of the old stuff will leave us).

I wasn't sure whether configobj is even maintained anymore, found the port to Python 3 only recently (Michael Foord's site seems to be unmaintained). I assume you're using this variant? https://github.com/DiffSK/configobj/blob/master/README.md

Yes, that's the one (and the one you get through pypi), looks well
enough maintained to me and didn't hit any bugs yet.

I'll have a look at the branch. What's the progress on that one?

The configobj part should be done, I'm working mostly on the doc there
(should have started a new branch I guess, but I started with generating
documention from the config file and then one thing lead to the next...).

---

Reply to this email directly or view it on GitHub:
#98 (comment)

[untitaker]

I'm currently rebasing.

[geier]

somehow this escaped me, could you please file issues for these crashes?
I have been using the configobj branch continously and didn't have any
issues.

Quoting Markus Unterwaditzer (2014-08-14 13:50:20)

@geier The configobj branch has some issues (crashing during usage), which is why i rather recommend adding CLI tests to the click branch and merging click first.

---

Reply to this email directly or view it on GitHub:
#98 (comment)

[untitaker]

They're mostly fixed on my PC.

[untitaker]

And I had to remove the subcommand feature again... click has another issue i can't fix.

[geier]

if this means we're now depending on a released version of click again,
I'll try to find time to write tests and docs tonight.

Quoting Markus Unterwaditzer (2014-08-21 14:30:57)

And I had to remove the subcommand feature again... click has another issue i can't fix.

---

Reply to this email directly or view it on GitHub:
#98 (comment)

[untitaker]

There's currently no release that contains both fixes. We can't work around any of them, at least not without manually modifying argv.

[untitaker]

3.2 was released, i removed the workaround since we depend on that version anyway.

[untitaker]

Are there any tasks left for this?

[geier]

There are some problems with color names, urwid uses other color
names
than click.

[untitaker]

Maybe we could just use the functions in khal.terminal for ikhal though? Unless there is a way to use urwid's color support without the rest of urwid...

[geier]

It looks like the current situation regarding the default command and options is rather messed up. With e.g. having calendar as the default command khal -a CALNAME works, but khal --days 3 does not. On the other hand khal calendar --days 3 works but khal calendar -a CALNAME does not.

As I said before, if there is no proper solution to this, I'm willing to scrap khal's abilitiy to run without any subcommands.

[untitaker]

Click has a very sharp differentiation between options for the whole group and options for subcommands, which means khal -a CALNAME calendar works, but khal calendar -a CALNAME doesn't. And the default command support for khal currently doesn't support passing extra args on to the default command itself.

So:

khal --days 3 doesn't work

That's because --days is not a global argument. The current support for default commands is a hack which can't pass options to subcommands.

khal calendar -a CALNAME doesn't work

That's because -a is registered as a global option, which means to click that it has to go before the command name.

[untitaker]

See also pallets/click#108

[geier]

Ah, I didn't get that. I'll update the docs.

[untitaker]

The situation with default commands is really idiotic IMO. Not sure if we even should support it if it's that confusing.

[geier]

I have experimentally re-enabled -a/-d for calendar and agend (see 7e1fb34), but its rather ugly. I'm still undecided if we should remove the default command.

[geier]

The current situation is not optimal anyway, as -d makes not much sense for new.

[untitaker]

Coming back to this: If we should accept both khal -d foo command and khal command -d foo, then i don't think click is the right choice for khal.

I reverted coloring for now, we could add it later.

[untitaker]

I removed the global -a and -d options and made them per-command. I think the solution is elegant, but the code is verbose.

[geier]

hmm, my last commit didn't really fix new, with khal new -a work 22:00 23:00 description I get this

set([u'k', u'r', u'o', u'w'])
Traceback (most recent call last):
  File "/home/cg/.virtualenvs/vdir/bin/khal", line 9, in <module>
    load_entry_point('khal==0.4.0.dev-27-gf5377d3', 'console_scripts', 'khal')()
  File "/home/cg/.virtualenvs/vdir/local/lib/python2.7/site-packages/click/core.py", line 609, in __call__
    return self.main(*args, **kwargs)
  File "/home/cg/.virtualenvs/vdir/local/lib/python2.7/site-packages/click/core.py", line 589, in main
    rv = self.invoke(ctx)
  File "/home/cg/.virtualenvs/vdir/local/lib/python2.7/site-packages/click/core.py", line 935, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/cg/.virtualenvs/vdir/local/lib/python2.7/site-packages/click/core.py", line 781, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/cg/.virtualenvs/vdir/local/lib/python2.7/site-packages/click/core.py", line 416, in invoke
    return callback(*args, **kwargs)
  File "/home/cg/workspace/khal/khal/cli.py", line 187, in new
    list(description)
  File "/home/cg/workspace/khal/khal/controllers.py", line 157, in __init__
    collection.default_calendar_name,
  File "/home/cg/workspace/khal/khal/khalendar/khalendar.py", line 235, in default_calendar_name
    return self._calnames.values()[0].name
IndexError: list index out of range

[untitaker]

It's fixed now, see commit msg for explanation.

[geier]

Otherwise this seems to work fine now. If you rebase it on master I'll merge it (or feel free to do it yourself).

[untitaker]

The Calendar class had a global _calendars list to check whether the calendar tables need to be created. I removed that and make khal simply check whether the tables exist, because it caused problems when running multiple testcases that are supposed to run isolated.