Skip to content

Fix uid: search to match by prefix instead of substring - #357

Open
harriiinnii wants to merge 3 commits into
lucc:mainfrom
harriiinnii:fix/uid-search-prefix-matching
Open

Fix uid: search to match by prefix instead of substring#357
harriiinnii wants to merge 3 commits into
lucc:mainfrom
harriiinnii:fix/uid-search-prefix-matching

Conversation

@harriiinnii

Copy link
Copy Markdown

Summary

khard list displays a short unique UID prefix for each contact (e.g. a for a contact whose UID starts with a and is the only one with that prefix). However, uid:a matched by substring, so it would match both aaabbbb and bbbaaaa instead of only aaabbbb. This made the displayed prefix unusable as a selector.

Problem

parse("uid:xxx") returned a plain FieldQuery("uid", "xxx"), which inherits TermQuery.match and does term in value.lower() — a substring check. The displayed prefix is a prefix, not a substring, so the two behaviours are inconsistent.

Solution

Add a UidQuery subclass of FieldQuery that overrides _match_union to use str.startswith for string values. Update parse() to return a UidQuery when the field is "uid".

The change is intentionally minimal: only the UID field is affected, and the rest of the query machinery (AndQuery, OrQuery, equality, hashing, __str__) works as before.

Testing

  • Added TestUidQuery in test/test_query.py covering: prefix match, substring-only non-match, exact match, case insensitivity, empty term with and without uid set, and disambiguation of two contacts sharing a common prefix.
  • Added test_uid_field_creates_uid_query_instance to TestParser verifying that parse("uid:xxx") returns a UidQuery.
  • All existing tests are preserved unchanged.

Fixes #327

khard list displays a short unique UID prefix for each contact, but
uid:xxx searched by substring so uid:a would match both "aaabbbb" and
"bbbaaaa". Introduce UidQuery which uses startswith so the displayed
prefix can be used directly to select a contact unambiguously.

Fixes lucc#327
UidQuery is a subclass of FieldQuery so isinstance always returns True;
use type() equality to assert that parse() returns a UidQuery specifically.

@lucc lucc left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks good, I just have some nitpicks.

Comment thread khard/query.py Outdated
Comment thread khard/query.py Outdated
Comment thread khard/query.py Outdated
Comment thread test/test_query.py Outdated
- Remove unnecessary backslash escapes from AndQuery/OrQuery docstrings
- Remove forward-reference quotes from UidQuery._match_union type hint
  (str | datetime | list | dict[str, Any] works unquoted on Python 3.10+,
  consistent with the rest of the file)
- Drop redundant type assertion in test_uid_field_creates_uid_query_instance;
  assertEqual(actual, expected) already verifies both value and type via
  UidQuery.__eq__

@lucc lucc left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I activated the ci pipeline so you can see that your changes break some other tests. Please run all test locally to speed up the feedback cycle.

The breakage is not severe, its just that some uid queries are being compared and their type now obviously changed. Please update the breaking tests as well.

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.

Restrict UID search query to prefixes, matching displayed ones

2 participants