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

Added autocomplete of mail addresses to attendees in event editor in interactive mode. #1324

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

dabrowskiw
Copy link

Hello,

I have added autocompletion of attendees in the event editor.
This works by exposing an additional configuration option "address_adapter" in the account section. The command specified there is run on startup using "bash -c" and the input is parsed as the contacts for that account. The parser (in CalendarCollection, in _contacts_update()) is written under the assumption that "address_adapter" will be "khal email | tail -n +2", however I am sure other contact managers can be forced to output in the same format, or the parser can be extended to be more intelligent.

The autocomplete (in attendeewidget.py) is using IndicativeListBox from additional_urwid_widgets (available via pip, pyproject.toml updated accordingly) to display autocomplete options and some ugly hackery to pretend that focus is still on the address line while it actually is in the list.

All tests run, and I suspect that the autogenerated configspec.rst should include the comment from khal.spec where I explained how address_adapter works - I am not sure though since I am not familiar with Sphinx and was not able to divine how the Makefile generated the configspec.rst file.

This code is not going to win any beauty contests, the best I can do is hope that only a few of those who look at it will need PTSD treatment. However, this is a development driven by need and not by too much spare time, so I am afraid that it's either "share it now" or "share it when it's cleaned up, which may well be never". If you think the usefulness of the feature outweighs the ugliness of the code, I am more than happy to contribute, but I will not be offended if you don't.

Anyways, thanks for the great tool that khal is, I very much enjoy working with it :)

# since it has only been developed with khard in mind - but other providers
# should be configurable to create the same output, or the parser could
# be extended (see _contacts_update in CalenderCollection in khalendar.py)
address_adapter = string(default=None)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What behaviour does None cause and why not default to khard email | tail -n +2?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None causes no autocompletion to be available (no address book). The idea behind not setting the default to khard email was that not everyone might have khard installed.

However, I agree that this is counterintuitive. I have committed a change that:

  • Adds an address_adapter field to the [default] section that
    • Is, by default, khard email | tail -n +2
    • Can be None, meaning "empty address book"
  • Adds the possible value "default" (which is also the default) to the address_adapter in the account section. That value causes the address_adapter from the [default] section to be used for that calendar
  • Tests whether the command from address_adapter was run successfully. If that is not the case, the effect is the same as setting address_adapter to None, i.e. the address book is assumed to be empty.

Does that make more sense?

@geier
Copy link
Member

geier commented Apr 27, 2024

Thanks for your contribution, @dabrowskiw! This sounds like a very useful feature!

Some notes:

For me, the attendee widget doesn't have a leading 'Attendee' anymore and I believe that text field should be labeled.

image

I would liket to drop the ruff commit, it adds a lot of noise which makes reviewing the PR harder. (I rebased this PR on master and dropped the ruff commit, see https://github.com/pimutils/khal/tree/rebase_pr1324 )

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.

3 participants