Re: image-transform.el and image-mode.el rewrite

[Vitalie Spinu]

 >>> Stefan Monnier on Thu, 18 Dec 2014 10:15:07 -0500 wrote:

 > - You seem to have rename a few "image-mode-FOO" to "image-BAR" and also
 >   a few "image-TOTO" to "image-mode-TITI".  I'm not necessarily opposed
 >   to those changes, but I'm not sure I understand the rationale behind
 >   it.  I think it would be good to document (e.g. in the "Commentary:"
 >   section) the convention used to decide whether the name should be
 >   "image-mode-FOO" or "image-BAR".

With the exception of `image-mode-fit-frame` all renames either added or
removed -mode- from the name. The rationale is very simple. Functions
that operate on images and are meaningful outside of `image-mode` don't
have -mode- in the name. Eventually, it would be nice to extract all
these functions into a separate `image-manip.el`.

The recently added image-mode-fit-frame was renamed into
image-scale-frame-to-fit-image because I wanted to place it on "s"
prefix in `image-manipulation-map' with all other scaling commands:

      "ss" 'image-scale-to-fit-window
      "sh" 'image-scale-to-fit-height
      "sw" 'image-scale-to-fit-width
      "sS" 'image-stretch-to-fit-window
      "sf" 'image-scale-frame-to-fit-image

The "f" in `image-mode-map`, which is arguably more suitable for this
functionality, has been used for `image-next-frame` for long time. So I
decided to use -scale- as a new mnemonic.

 > I recommend you take a look at doc-view.el and plan on sharing some
 > of the code there as well. E.g. the image-scale-step should probably
 > be merged with doc-view-shrink-factor.

Eventually all of the doc-view scaling and fitting should be based on
image-manip. I plan to look into that once this patch is done with.

 > - The :flatten arg to image-transform looks wrong.  Instead of
 > ":flatten" it should be ":flatten t" (i.e. all keywords should be
 > followed by a value and a nil value should be equivalent to not
 > having mentioned the keyword).  IOW the "Boolean specs can miss the
 > value, in which case t is assumed" is a misfeature.

It mimics convert/bash specification where boolean parameters are either
there or not. I would be happy to remove this feature, as it brings some
additional complexities.

 > - The image-transform-features:convert seems over-engineered.  I think
 >   I'd rather have a ":convert-args" that provides a mechanism to pass
 >   any args to "convert" and then remove the bulk of those keywords.

The `image-transform-features:convert' in the current form is at the
core of image-transform.el. Convert specification is used as a canonical
description of the transformations, thus ensuring compatibility between
backeds. This allows for writing of generic code independently of the
applied backend. Currently, any transformation chain that works under
'native backend will work with 'convert. If 'native backend is extended
in the future with more transformations, or new backends are added, old
code will work happily with all of the new features.

Also, a comprehensive list of transforms and their types makes possible
interactive application of transforms as in `image-add-transform`. This
essentially makes from Emacs a lightweight image editor.


  Vitalie