mikf / gallery-dl

Command-line program to download image galleries and collections from several image hosting sites
GNU General Public License v2.0
10.66k stars 880 forks source link

[Configuration] `path-metadata` passing wrong path in `keywords` #5752

Closed shrublet closed 1 week ago

shrublet commented 1 week ago

I was testing the keywords-eval feature with path-metadata, and came across an issue in which the path passed by path-metadata passes the path for the previous file (referred to as path_ below).

For example, in my config (yaml), i have a keyword entry as follows (Path from pathlib.Path):

test_keyword:  "\fE Path(str(path_))"

On the other hand, passing path_.directory will result in the correct directory:

test_keyword_2:  "\fE Path(str(path_.directory))"

I'm not exactly sure why this happens, as when running a post-processor, the path_ found in the metadata dictionary in that case is correct. I hope this is replicable and not just an issue with my config.

mikf commented 1 week ago

keywords get evaluated before building the accompanying filename for this set of metadata, similar to the prepare event (even earlier than that, but nothing significant happens between the two), so the metadata-path object still contains the previous filename at that point in time.

The target directory gets built separately and even earlier, so its value is the correct/current one.

keywords values are kind of meant to be used during filename creation, so I'm not sure if moving the eval step after would be a good idea. Maybe it should be tied to a processor event and made configurable. Maybe I should add another event directly after building filenames. Not quite sure how best to solve this.

shrublet commented 1 week ago

Lest it gets overly convoluted for little gain, I guess it's okay to write it off as a non-issue to be honest, and I'll go ahead and close this issue. It's fairly edge-case at least, and I think adding an event, while useful, might be over complicating a simple feature. Or that's my two cents.

The logical order of how things are evaluated makes a lot of sense after explanation. My use-case can get away with just getting the directory component from the path anyways. It was just really confusing as to why the directory was different from the full path. I'd hope they'd at least both be "wrong" or both be "right", but in this case, I benefit from them being handled separately. Thank you for the quick response as always.