[postprocessor:mtime] add 'value' option

[bradenhilton]

Closes #2721.

I'm not at all familiar with reStructuredText, so I based a lot of the doc changes on other options.

I'm also not sure if setting self.value_fmt and then setting self._value_fmt is necessary, again I saw another place where something similar was done so I went with it for now.

I was also considering wrapping the value of mtime.key in braces as the default format string for mtime.value, and using it as the only value for mtime. Currently it defaults to an empty string.

A review is welcome.

[mikf]

It would be better to put most of the work into __init__() and only do the absolute necessary in run(). I'd parse value once if given, and also pack the operation for key into a function so that we only have to do a single function call in run() to fetch the mtime value from kwdict.

Something like this in __init__(), and in run() we only do mtime = self._get(pathfmt.kwdict):

        value = options.get("value")
        if value:
            self._get = formatter.parse(value, None, util.identity).format_map
        else:
            self._get = operator.itemgetter(options.get("key", "date"))

This would also need an import operator at the top, and it now causes an error if a field with name key is not defined, instead of silently doing nothing like it did until now. Maybe it would be better to wrap this into a lambda that calls .get().

        else:
            key = options.get("key", "date")
            self._get = lambda kwdict: kwdict.get(key)

I'm not at all familiar with reStructuredText, so I based a lot of the doc changes on other options.

You should make the underline as long as the heading itself:

mtime.value
---------

and I'd rather add a note to mtime.key saying

Note: This option gets ignored if `mtime.value`_ is set.

than what the last line says.

The default value should be noted as plain null, I think.

[bradenhilton]

Just commenting to acknowledge that the checks have failed.

[mikf]

Concerning the failed tests, you can just remove the 3 lines here, or at least the assert statement.

gallery-dl/test/test_postprocessor.py

Lines 353 to 355 in 90ae48c

	def test_mtime_default(self):
	pp = self._create()
	self.assertEqual(pp.key, "date")

If you for some reason feel like it, you can add some additional tests for value, but that is really not important.

To run those tests locally, use make test or python test/test_postprocessor.py

[bradenhilton]

Are those test changes acceptable?

[mikf]

They are. Thank you very much.

[left1000]

Some of this jargon is over my head, but does this merge address that fact that all of my postprocessors fail to preserve the date modified of the base post that they're related to? Like I download an image, the date becomes the date on the website's server of the date the artist uploaded the image, then I run a postprocessor to save the description that the artist wrote along with that image, but this description just uses the current date on my computer instead of remembering the image's date?

Or is that not what this is about?

If that is what this is about, what's the syntax I have to add to my postprocessor to enable this new date saving feature?

[mikf]

@left1000 This PR is about adding a more flexible way of getting a value for the mtime post processor.

What you want is to enable the mtime option for your metadata post processors, so that all files generated by them (descriptions, etc.) get the same timestamp as the actual file.

{
    "name": "metadata",
    "mtime": true,
    "...": "..."
}