Add capture month to collection and collection filters

[da-phil]

This PR adds the photo capture month to the "collections" and "collection filters" widgets, as requested on pixls: https://discuss.pixls.us/t/filtering-collection-on-capture-date-but-by-month-only/51621

This will enable users to better filter and analyze their photos based on the time of capture.
My typical use-case for this is finding suitable calendar pictures, given that I want them being taken in the same month as they should appear in the calendar.

A typical example for me is finding winter pictures, where I just want to filter for photos taken in December, January and February:

I have the following question to reviewers and other users:
Do you think we need the names of the months in there, or would numbers 00-12 be sufficient, given the necessity to translate those month names and their 3 character abbreviations?

Disclaimer: this code was co-created with Claude.

[wpferguson]

You need to add DT_COLLECTION_MONTH_PROP to the Lua enums at the end of src/collect.c

[da-phil]

Thanks for the initial review @wpferguson @ralfbrown @TurboGit. Will address them once I'm getting back to the code.

Until then I have the following general question to you:
Do you think we need the names of the months in there, or would numbers 00-12 be sufficient, given the necessity to translate those month names and their 3 character abbreviations?

[TurboGit]

Do you think we need the names of the months in there

I think it is fine to have the months name here. Those could be reused anyway in other contexts.

[wpferguson]

Tested, works with Lua

[da-phil]

I just completed some intensive testing with both "collections" and "collection filters" without encountering any issue or regression. Is there anything specific I shall watch out for?

[wpferguson]

Just tested. Lua is broken.

    35.5954 LUA ERROR: dtutils.lua: prequire: 223: Error loading test_scripts/test_month_collection 
    35.5958 LUA ERROR: dtutils.lua: prequire: 224: Error returned is luaA_enum_to: Enum 'dt_collection_properties_t' field 'DT_COLLECTION_PROP_MONTH' not registered! 
    35.5962 LUA ERROR: script_manager.lua: activate: 543: error loading test_scripts/test_month_collection 
    35.5966 LUA ERROR: script_manager.lua: activate: 544: error message: luaA_enum_to: Enum 'dt_collection_properties_t' field 'DT_COLLECTION_PROP_MONTH' not registered! 

[da-phil]

Just tested. Lua is broken.

    35.5954 LUA ERROR: dtutils.lua: prequire: 223: Error loading test_scripts/test_month_collection 
    35.5958 LUA ERROR: dtutils.lua: prequire: 224: Error returned is luaA_enum_to: Enum 'dt_collection_properties_t' field 'DT_COLLECTION_PROP_MONTH' not registered! 
    35.5962 LUA ERROR: script_manager.lua: activate: 543: error loading test_scripts/test_month_collection 
    35.5966 LUA ERROR: script_manager.lua: activate: 544: error message: luaA_enum_to: Enum 'dt_collection_properties_t' field 'DT_COLLECTION_PROP_MONTH' not registered! 

How can I reproduce your test?

[wpferguson]

Here's the script,

test_month_collection.lua.txt

create a new directory in your lua scripts named test (or whatever) and drop the script in there after removing the .txt extension. When you start it, it will filter for images taken in April. Stop it before you exit, so that it doesn't try and start again when you start darktable.

The issue is at the end of src/libs/collect.c where the Lua enums are declared. Somehow it got lost in your latest round of changes/testing.

[da-phil]

Thanks, I can now reproduce the same error, but only while I was running a darktable binary w/o this change. With this change it works.

The issue is at the end of src/libs/collect.c where the Lua enums are declared. Somehow it got lost in your latest round of changes/testing.

Hmmm, it is still there:
https://github.com/darktable-org/darktable/pull/20697/changes#diff-2c4ad8c3aa04d783a05735ec0fbeb46a420d90c81153134287e320ff0a52f36bR4066

[wpferguson]

Oops, maybe my error... Testing too may collection PRs....

EDIT: It works correctly. Sorry for the noise.

[TurboGit]

Works for me, code ok now. Thanks!

[MStraeten]

this breaks old filmroll sort by folder.
instead of the folder now just the shown subfolders are taken into account for sorting.

now if 2 levels are selected and order by folder
/volumes/aa/2010 and /users/bb/2012 are sorted as
aa/2010
bb/2012

instead of former
bb/2012
aa/2010
which took the whole folder path into account

[da-phil]

this breaks old filmroll sort by folder. instead of the folder now just the shown subfolders are taken into account for sorting.

now if 2 levels are selected and order by folder /volumes/aa/2010 and /users/bb/2012 are sorted as aa/2010 bb/2012

instead of former bb/2012 aa/2010 which took the whole folder path into account

Ups, sorry, I have never used filmrolls before, hence it slipped through.
I'll take a quick look and see if I can fix it, if not, we can revert the PR for the time being.

[da-phil]

@MStraeten I cannot reproduce and I'm not sure if my changes have any influence on filmroll sorting.
Are you sure that this change was not done by #20718 which was also merged a couple of hours ago?

[da-phil]

The issue seems to be in _sort_filmroll_rows at collect.c:167-172. It was sorting by ra->folder which is the truncated display name (only the last N folder levels from dt_image_film_roll_name(), instead of the full path stored in ra->value.

With show_folder_levels set to 2:

/volumes/aa/2010 → display name aa/2010
/users/bb/2012 → display name bb/2012

Sorting by the display names gives aa/2010, bb/2012, but sorting by the full paths gives /users/bb/2012, /volumes/aa/2010 — the old behavior you expected.

Does the following patch help?

diff --git a/src/libs/collect.c b/src/libs/collect.c
index 139f9b2cc0..f2aac24254 100644
--- a/src/libs/collect.c
+++ b/src/libs/collect.c
@@ -168,7 +168,7 @@ static gint _sort_filmroll_rows(gconstpointer a, gconstpointer b)
 {
   const filmroll_row_t *ra = a;
   const filmroll_row_t *rb = b;
-  return g_ascii_strcasecmp(ra->folder, rb->folder);
+  return g_ascii_strcasecmp(ra->value, rb->value);
 }

[MStraeten]

sorry, you're right, i haven't seen the other collection affecting merge -
the patch just repairs the order, but the i'ts not possible to reverse the sorting.

[MStraeten]

i will file an issue for that so it can be properly tracked ...