afh commented 2 years ago

This is a first, likely incomplete, attempt to add a kind column in [the] contact table as mentioned in the todo.txt.

I haven't been able to get the tests running with my nixpkgs setup on macOS due to errors (see below). Being unfamiliar with mypy any comment that might be helpful is appreciated.

% nix-shell -p nsh python3 python3Packages.unidecode python3Packages.ruamel-yaml python3Packages.vobject python3Packages.atomicwrites python3Packages.configobj python3Packages.setuptools python3Packages.pip pylint mypy --run "mypy khard"

``` khard/carddav_object.py:20: error: Library stubs not installed for "atomicwrites" (or incompatible with Python 3.9) khard/carddav_object.py:20: note: Hint: "python3 -m pip install types-atomicwrites" khard/carddav_object.py:20: note: (or run "mypy --install-types" to install all missing stub packages) khard/carddav_object.py:23: error: Skipping analyzing "vobject": module is installed, but missing library stubs or py.typed marker khard/address_book.py:10: error: Skipping analyzing "vobject.base": module is installed, but missing library stubs or py.typed marker khard/address_book.py:10: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports khard/address_book.py:10: error: Skipping analyzing "vobject": module is installed, but missing library stubs or py.typed marker khard/config.py:11: error: Skipping analyzing "configobj": module is installed, but missing library stubs or py.typed marker khard/config.py:16: error: Skipping analyzing "validate": module is installed, but missing library stubs or py.typed marker khard/khard.py:288: error: Argument 1 to "append" of "list" has incompatible type "methodcaller"; expected "attrgetter[Any]" khard/khard.py:290: error: Argument 1 to "append" of "list" has incompatible type "methodcaller"; expected "attrgetter[Any]" khard/khard.py:445: error: Incompatible types in assignment (expression has type "TermQuery", variable has type "AnyQuery") khard/khard.py:448: error: Incompatible types in assignment (expression has type "AndQuery", variable has type "AnyQuery") Found 10 errors in 4 files (checked 14 source files) ```

afh commented 2 years ago

I probably should have made this a draft pull request…

lucc commented 2 years ago

mypy and tests

The tests are also broken on the develop branch, I am working on it in #307. Some of your errors are already fixed in the github-actions branch but when you use nix you have to build a python env with the python packages you want, just installing them side by side does not work. Check https://nixos.org/manual/nixpkgs/stable/#python

With nix-shell --pure -p python3.withPackages' (p: with p;[mypy ruamel_yaml vobject configobj unidecode])' --run "mypy khard" I still get

khard/carddav_object.py:23: error: Skipping analyzing 'vobject': found module but no type hints or library stubs
khard/carddav_object.py:1275: error: Argument 1 to "yaml_clean" has incompatible type "object"; expected "Union[str, Sequence[Any], Dict[str, Any], None]"
khard/address_book.py:10: error: Skipping analyzing 'vobject.base': found module but no type hints or library stubs
khard/address_book.py:10: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
khard/address_book.py:10: error: Skipping analyzing 'vobject': found module but no type hints or library stubs
khard/config.py:11: error: Skipping analyzing 'configobj': found module but no type hints or library stubs
khard/config.py:16: error: Skipping analyzing 'validate': found module but no type hints or library stubs
Found 6 errors in 3 files (checked 14 source files)

but with --ignore-missing-imports it is only

khard/carddav_object.py:1275: error: Argument 1 to "yaml_clean" has incompatible type "object"; expected "Union[str, Sequence[Any], Dict[str, Any], None]"
Found 1 error in 1 file (checked 14 source files)

your PR

Firstly: Thank you :)

I think you do not need a config option "preferred_kind" as the standard says

Implementations MUST support the specific string values defined above. If this property is absent, "individual" MUST be assumed as the default.

And if a config option would actually have been needed it needs to be added to the config specification file.

Even if some upstream tests are broken you can still implement tests for your new code.

afh commented 2 years ago

Thanks for your feedback, I'll work it in. Regarding tests it seemed to me that tests did not run at all due to the errors I saw, yet that could well be my unfamiliarity with mypy; I'll look into it.

lucc commented 2 years ago

mypy does not run the tests. It is only a type checker. Use python setup.py test to run the tests. You can also have a look at the travis config file for the exact commands that would be run in CI.

afh commented 2 years ago

Thanks for your helpful explanations, @lucc. I went ahead and fixed the tests in regards to the newly added kind column being displayed.

My preference would be to merge this PR once you find it acceptable and work on the kind editing and filtering in a separate PR (#310), but I'm happy to do what you think works best for you.

ℹ️ I'm uncertain whether the following is an issue only with my setup, yet the test_contact_is_found_if_name_matches (test.test_command_line_interface.AddEmail) ... test hangs, i.e. is unresponsive and does not finish; I let it "run" for a minute and decided then to ^C it…

lucc commented 2 years ago

I am unsure if we should display the kind attribute by default. My reasoning goes like this:

I assume that most users would have only "individual" vcards in their address books
"individual" is the default if no kind attribute is present on the vcard. This means we add noise to the output if the attribute is not set.
if only "individial" vcards are in a listing it adds very little info to display this.

I have two ideas to improve this:

a config/command line option to show the kind
a similar logic like with the address book column: only show it if not all cards are "individual" (the address book column is only shown when the output contains cards from more than one address book but I would also show the kind if all cards in the output have the same kind but that kind is not "individual")

What do you think?

afh commented 2 years ago

I think your idea to improve displaying the kind column conditionally and via a config/command-line option makes sense. I'll look into it. Thanks for the suggestion :)

afh commented 2 years ago

@lucc, this should be ready for another look. I went ahead and added a new config option (show_kinds similar to show_uids) allowing to configure whether the kind column should always be shown (defaults to no) as well as displaying the kind column conditionally whether the result list consists entirely of or contains contacts with non-individual kind.

lucc commented 2 years ago

Lets move on to #310 :)

afh commented 2 years ago

Thanks for merging, @lucc 😃

lucc / khard

Add kind column in contact table #309

mypy and tests

your PR