Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

thumbnails: Tolerate absence of MTime and URI #230

Conversation

chrysn
Copy link
Contributor

@chrysn chrysn commented Jan 31, 2024

A change to the Thumbnail specification has recently been accepted that allows MTime and URI to be missing, in which case the thumbnail is regarded as always-matching.

To be precise, that change only altered the behavior for shared thumbnails, but that distinction is not made in pqiv so far, and I see no harm in accepting thumbnails with those properties if the creator saw fit to violate the thumbnail specification in that very precise way.

This change enables the use of montage mode on git-annex repositories where only the shared thumbnails are locally available (or other sparsely checked out media where the file can not be stat()ed but is seen in the listing). Further changes may enhance usability in such scenarios (for example, the -1 family of arguments could be allowed in montage mode, and bound by the user to a command to actually fetch the image; behavior on entering an absent file could be altered to avoid that once you go back to montage mode the file doesn't show any more) -- but at any rate, this is a start, and is also beneficial for fully checked out media (eg. where MTime is just not reliable, but the creator had means of keeping shared thumbnails current).

Details for the review:

  • A sentinel time_t was introduced for practical reasons. If we want to insist that thumbnails with a present MTime of 1970-01-1T00:00:00Z is not accepted for a file whose MTime we can not determine, we could pass a nullable pointer to a time_t.
  • The error path after finding a non-matching URI or MTime is now a tad faster because the code can err out as a mismatch was found. This is just a side effect; I focused on simplicity of the change, I don't consider that speedup relevant as it affects just the error case.

This applies the changes from [51] where rationale for that change is
given.

[51]: https://gitlab.freedesktop.org/xdg/xdg-specs/-/merge_requests/51
@phillipberndt phillipberndt merged commit 8af563e into phillipberndt:master Feb 16, 2024
@phillipberndt
Copy link
Owner

Thanks, looks good to me. Merged.

@chrysn chrysn deleted the tolerate-mtime-uri-absence branch February 16, 2024 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants