search

pimutils / todoman

✅ A simple, standards-based, cli todo (aka: task) manager.
https://todoman.readthedocs.io
ISC License
482 stars 76 forks source link

fix filtering for non-ASCII categories #541

Closed balejk closed 10 months ago

balejk commented 10 months ago

To make category filtering case insensitive, todoman converts the category provided on the commandline to uppercase, uses the sqlite upper function to convert the cache entries and then compares the converted strings.

However the sqlite upper function only works for ASCII strings causing problems when the category is for instance ý, which sqlite translates to uppercase as ý while the correct version is Ý, causing the filtering for this category not to return any tasks.

Add a new column (category_upper) to the categories table with the uppercase version of the category name converted by Python (which performs the conversion correctly) and use this for filtering while keeping the original version in the category column as before.

Add a corresponding test.

Fixes #540.

balejk commented 10 months ago

Is there a way to force cache regeneration? As it is now, this causes "column does not exist" error after update.

WhyNotHugo commented 10 months ago

Just delete the cache and it'll be regenerated.

If you bump Cache.SCHEMA_VERSION, todoman will recognise all caches as old and regenerate them automatically. You should increase the SCHEMA_VERSION to 10 for this PR so the cache is regenerated seamlessly for users when upgrading.

balejk commented 10 months ago

Just to be sure: Д is uppercase, right? I definitely want to keep an uppercase Cyrillic letter in tests.

I believe it is.

balejk commented 10 months ago

Just delete the cache and it'll be regenerated.

Yes, of course, I meant for production.

If you bump Cache.SCHEMA_VERSION, todoman will recognise all caches as old and regenerate them automatically. You should increase the SCHEMA_VERSION to 10 for this PR so the cache is regenerated seamlessly for users when upgrading.

I will push this in a second.

balejk commented 10 months ago

Just to be sure: Д is uppercase, right? I definitely want to keep an uppercase Cyrillic letter in tests.

But actually, I think you rather want to make sure there is such a lowercase letter, no?

WhyNotHugo commented 10 months ago

But actually, I think you rather want to make sure there is such a lowercase letter, no?

I wanted to make sure that we have both lowercase AND uppercase in tests.

WhyNotHugo commented 10 months ago

Thanks!

WhyNotHugo commented 10 months ago

@balejk You didn't add yourself to AUTHORS.rst.

balejk commented 10 months ago

@WhyNotHugo I see, sorry. Feel free to add me if it's required (or I can open another PR, but that seems needlessly complicated).