Bulk Export: Add CATEGORY_EACH variable and - #22318
clawoflight wants to merge 5 commits into
Conversation
|
It might be a good idea to figure out if it's a bug, a regression, a misunderstanding, etc before jumping to fix it. If it's a regression, then we are just slapping a band aid over the problem to hide it rather than fix it. This can sit, but #22317 needs to be understood so that we know what to fix. |
|
I tested using the following tags and tagged 6 image
I used the variable line The result I expected was darktable_exported/Allen/Jane - 6 images What I got was darktable_exported/Allen/Jane/Jr - 6 images |
Thank you so much for taking a look so quickly! This is not a regression. It seems to be a feature gap that was never covered. #12468 In the code, I couldn't find any sign that this ever was intended to work (which is why I had to make some functions return GList* instead of strings). If anything, the existing behavior was the band-aid :) |
|
Thank you for your test. My current design used a Cartesian product over the values, which makes sense for independent tags, but is unintuitive for nested hierarchies. I can think of 2 ways to make this work. Which do you prefer:
Edit: I pushed approach 2 in appended commits so you can play with that behavior. I will of course clean up the history once we decided how to proceed :) |
9ec3f1c to
45ff89d
Compare
|
My desired output was Now I have a question about blast radius. Variable substitution is used in more that just the exporter. It's also used in thumbnail overlays, darkroom image information, etc. Look through the preferences for anywhere strings use variable substitution. We need to make sure those don't break with multiple string results and how to display them. This may be why the categories got joined with a comma instead of being split into multiple strings. |
|
The blast radius should be minimal: Only the new In other contexts, CATEGORY_EACH behaves like CATEGORY. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Core list expansion and disk export behavior still contain correctness and memory-safety defects.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (5)
What changed in this PR
Adds disk bulk-export support for creating one output path per hierarchical tag value.
Changes:
- Adds
CATEGORY_EACHand multi-path variable expansion. - Updates disk exports to process multiple destinations.
- Adds tests, UI completion, and release documentation.
| File | Description |
|---|---|
| src/tests/variables.c | Tests list-valued path expansion. |
| src/imageio/storage/disk.c | Exports images to multiple expanded paths. |
| src/gui/gtkentry.c | Adds variable completion help. |
| src/common/variables.h | Declares the multi-path API. |
| src/common/variables.c | Implements list-valued expansion. |
| src/common/tags.h | Declares tag-list helpers. |
| src/common/tags.c | Retrieves hierarchical tag paths. |
| RELEASE_NOTES.md | Documents the new export variable. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Returns, for every attached tag under a category, the components below that category as a NULL-terminated array. The multi expander needs the whole tag, not the distinct subtags at one level, so that repeated references to the same category can walk one tag's path. dt_tag_get_subtags() is reimplemented on top of a new dt_tag_get_subtags_list(), which also fixes its substring-based de-duplication and orders the subtags by name.
CATEGORY collapses every matching subtag into one comma-separated string, so an image tagged Person|John and Person|Jane exports into a single "John,Jane" directory instead of one directory per person. Add CATEGORY_EACH[n,category], which yields the level-n component of each matching tag, and dt_variables_expand_path_multi(), which expands a path pattern once per tag. References to the same category select fields of one tag and so walk that tag's path, so $(CATEGORY_EACH[0,Person])/$(CATEGORY_EACH[1,Person])/$(CATEGORY_EACH[2,Person]) yields Allen/Jane, Allen/John/Jr and Allen/John/Sr. Distinct categories stay independent axes and still multiply. The product is capped at 1024 combinations; beyond that the expansion fails and the caller is expected to abort. The expander stays single-valued: the multi API owns the enumeration. Outside it CATEGORY_EACH collapses to the comma-joined scalar, so it is harmless in every other expansion path.
The file-on-disk storage expanded the filename pattern once, so a pattern with $(CATEGORY_EACH) only ever produced the comma-joined scalar. Use dt_variables_expand_path_multi() and loop over the paths it returns, writing the image once per tag. The mkdir, writability and conflict handling now run per output, so a skipped output no longer skips the whole image. Paths are still decided under the plugin lock, but the exports themselves run after it is released: dt_imageio_export() takes the same lock and would deadlock.
45ff89d to
70b2819
Compare
|
@wpferguson : Do you confirm that your expected output is now what we get with this PR? We introduce a new tag |
@TurboGit Salut! Only the handling of each path is backend-specific - I figured this wouldn't make sense for most backups, except maybe the gallery. The iteration could maybe be moved to |
Wouldn't that mean to call the export multiple times with identical parameters although only the export path/filename has changed? |
|
@TurboGit the export works correctly and I get the expected results. In my contrived example, each image is exported multiple times (6 images x 3 tag combinations = 18 images). I don't know if there is a way to export once and copy twice that makes sense. I could see the combinations of tags getting out of hand, especially if you make a mistake, and exports taking a lllllooooonnnnnnngggggg time. I don't know if multiple results makes sense in any context except export. I say that and as I'm saying it I think of one. I could see an image overlay with everyone that is tagged being shown so that you know who you tagged and who you didn't. But, how do we show it and how do we handle it? Return a comma separated string with the results and use ellipsis when the string overflows the overlay? So CATEGORY_EACH would have to be aware of how and where it was being used and return the context correct result. EDIT:
I think CATEGORY would do this, so this is probably already handled. |


Summary
This allows proper bulk export using hierarchical tags.
Previously, if an image had multiple tag values, you would get one directory for every combination of values.
Now,
$(CATEGORIES_EACH[])gives you one directory per tag value.This makes it easy to, e.g., export images into directories for people, or scene types (where each image could reasonably contain more than one).
This applies to the disk backend only. I can add gallery if wanted, the others probably don't make sense at all.
Referenced issue
Closes: #22317
Checklist
coding style.
own.
src/tests/integration/where thepixelpipe is touched, or
darktable-clias a headless smoke test._(), new preferences are registered indata/darktableconfig.xml.in.RELEASE_NOTES.mdentry was added.Test instructions
src/tests/variables.cclearly show the intended usePerson|JaneandPerson|John.$(CATEGORY[0, Person]): This expands to one file in "Jane, John"$(CATEGORY_EACH[0, Person]): This expands to two files, one in "John", one in "Jane"AI assistance
This MR was assisted by Deepseek V4 Flash via OpenCode. Everything was tested manually in addition to the unit tests.
Scope
We could easily add the following if we want, now that variables.c uses
GList *instead ofchar *:TAGS_EACHIMAGE.TAGS.HIERARCHY_EACHLABELS_EACH