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

[Stefan Monnier]

>   https://github.com/vspinu/image-transform/compare/emacs...master

Thanks Vitalie, overall it looks pretty good and I think it's pretty
much ready for inclusion.

This is not my area of expertise, so hopefully someone else can take
another look, but besides Michael's comments I'll add:

- 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".

- While it's OK to install such a change only for image-mode.el
  (i.e. take it one step at a time), 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.

- Use (require 'cl-lib) instead of (require 'cl-macs).

- You don't need to (require 'pcase), it's autoloaded.

- Don't use `(lambda ...) when you could use a closure instead.

- Use `define-error' instead of (put 'next-backend 'error-conditions ...).

- Use "convert" instead of (executable-find "convert"), especially since
  you define the variable to hold a string, and executable-find can return nil.

- (executable-find "convert") does not return a "path" but a "file name".
  A "path" is a list of directories, as in $PATH, load-path, ...

- 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.

- 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.


        Stefan