getkirby / ideas

This is the backlog of ideas and feature requests from the last two years. Use our new feedback platform to post your new ideas or vote on existing ideas.
https://feedback.getkirby.com
20 stars 0 forks source link

Plugin: allow changing filenames of file versions #300

Open fabianmichael opened 5 years ago

fabianmichael commented 5 years ago

When writing a custom a thumbnail driver, there’s currently no other option than overriding the file::version component for altering the filename of e.g. a thumbnail file. This could easily collide with other plugins. It would be great, if the file::version component or Kirby\Cms\Filename class would offer an API for altering the filename, that can be used by other plugins.

Use case:

  1. ImageDriver Plugin allows the developer to perform additional filters on an image file, e.g. a pixelate filter. If takes the pixelate option in the thumb method:
    echo $image->thumb(['width' => 300, 'pixelate' => 10)->html();
  2. ImageOptimizer Plugin allows the developer to optionally optimize images using a Cloud-Service or CLI-Tool like pngquant by setting the pngquant and palette options in the thumb method for outputting an 8-bit PNG file.
    echo $image->thumb(['width' => 300, 'pixelate' => 10, 'pngquant' => true, 'palette' => 16]);

Both plugins should be able to add their parameters to the output filename of the thumbnail, e.g.:

mountains-q85-pix10-pngquant-c16.png

If the Filename class would fire a hook when compositing the finals class name, this would easily be possible to allow plugins to take control of what parameters they need to have in the final filename.

Another option would be to do this automatically by converting the array of thumb options into a string (by @lukasbestle):

  1. The currently included attributes (dimensions, crop, blur, bw, q) are included in the filename like in the current implementation to preserve backwards-compatibility.
  2. Those attributes are then removed from the array of passed attributes.
  3. The remaining (= custom) attributes are sorted by key alphabetically to ensure reproducible filenames no matter in which order the attributes were passed.
  4. Depending on the value of the attributes, the resulting string is automatically determined like so: If the value is true, just include the key of the attribute. If it is false or null, don't include it at all. Any other value is just appended to the key (like for the blur value right now).
  5. The custom attributes are appended to the end of the filename after the standard attributes.
lukasbestle commented 5 years ago

I like the idea of a hook actually. It's quite a bit easier to implement but more flexible for plugins. It could look like this:

function($attributes) {
  if (isset($attributes['palette'])) {
    return 'c' . $attributes['palette'];
  }
}

All hook return values that are not null would then be imploded with dashes and appended to the attributes.

fabianmichael commented 5 years ago

@lukasbestle Does our current hook system allow us to set a priority for callbacks? Not really important in this case, but generally very handy, once you open the queue for plugins.

fabianmichael commented 5 years ago

Just a more complete example:

Kirby::plugin('megaimagecorp/optimizer3000', [
  'hooks' => [
    'filename' => function($attributes) {
      if (isset($attributes['palette'])) {
        return 'c' . $attributes['palette'];
      }
    },
  ],
]);

Already feels kind of right to me! As most OSs have a maximum length for file names (seems to be 255 in macOS on HFS+/APFS and most others), we could use a hash of that string, if the total length o the filename will exceed this maximum length. I don’t know how realistic this is and if it’s worth the extra complexity.

lukasbestle commented 5 years ago

Does our current hook system allow us to set a priority for callbacks?

No, I don't think so. But I think that priorities are often not a great solution if compared between different plugins (each plugin would want the highest priority, they won't really know if they are more important than another plugin).

As most OSs have a maximum length for file names (seems to be 255 in macOS on HFS+/APFS and most others), we could use a hash of that string, if the total length o the filename will exceed this maximum length.

The question is whether there will be that long file names at all. If there are only a handful of plugins in use and each one has 1-2 attributes that each only have a few letters, it should probably be alright.

A hash would greatly reduce the human-readability and debugging options. So it should only be the last resort. Maybe educating about this issue in the docs (so that plugin devs keep their attributes short) would help already.


One thing we need to take care of: Reproducible attribute order. Maybe the return values from the hook could be thrown into an array, the null values could be filtered out and then the array could be sorted alphabetically before imploding it with dashes. But I'm not sure if that results in a 100 % reproducible filename.

Also one detail: The hook should be able to return an array of attributes to include.

fabianmichael commented 5 years ago

@lukasbestle I don’t think, the hash alone will be a problem. But combined with a long file name. but I’m probably overengineering at this point. ;-)

Priorities for hooks are not really important for this feature, they’re more of a thing for a things like transforming sth. like KirbyText: lets say for instance, we got one plugin that replaces text variables with some strings, a second one, that applies hyphenation by inserting sort hyphens into words, and a third one, that transforms all special characters into HTML entities.

The plugins must run in the same other as described, otherwise the whole thing will not work correctly, as the hyphenator does not work with html entities, but we also want text variables to become hyphenated. Priorities can help a lot here. But you’re right—they’re far from being a perfect solution.

lukasbestle commented 5 years ago

The plugins must run in the same other as described, otherwise the whole thing will not work correctly, as the hyphenator does not work with html entities, but we also want text variables to become hyphenated. Priorities can help a lot here.

I don't think so to be honest. As I wrote above, the relative priority between different plugins that don't know about one another is a bit like z-index in a stylesheet where you don't know how other components were developed.

In your example, at least two of the three plugins would need to know that there are "more important" plugins. Which means: The plugins need to know each other (= the plugin devs need to write special rules for interactions with all other possible plugins).

Otherwise (with a simple z-index-like priority), it would result in this:

The only thing that could solve this issue is a configuration option for the order of the hooks that can be set by the site's dev (so they would need to integrate the plugins and decide on their order in the use-case they are actually used in).

But that's a bit off-topic for this issue. Maybe we should move this discussion to another idea issue.

fabianmichael commented 5 years ago

@lukasbestle Maybe we should just stop, as long as this is not a real issue to anyone! :-)

fabianmichael commented 5 years ago

What I almost forgot, while we’re at this: This would be a good opportunity to allow file type conversions through plugins. Typical use case would be to convert JPEG and PNG images into WebP and animated GIF/APNG into mp4.