emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Michael Heerdegen
Subject: Re: image-transform.el and image-mode.el rewrite
Date: Thu, 18 Dec 2014 15:17:01 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/25.0.50 (gnu/linux)

Vitalie Spinu <address@hidden> writes:

> I have finally got down to this.
> [...]
> At this stage the stuff is ready for a review as I have only minor
> things to fix. I will be traveling till 18th and will resume once I am
> back. Would be really great if interested people could have a quick look
> till then.

Looks like it was a lot of work - many thanks!

I can't speak for Emacs dev, hope someone with background knowledge can
also review your code.  I played with it a bit, and it was working
nicely in general.

Some comments:

- When I visit an image file, `image-manipulation-map' doesn't seem to be
  installed.  E.g. "+" is not working etc.  OTOH when I hit C-c C-c two
  times, it works then.  Is this expected?

- I think it would be good when there would be entries in the
  "Manipulate" menu (defined in `image-manipulation-map') for increasing
  and decreasing the image size.

- I see some warnings about unused lexical variables.  I think some are
  concerning pcase (where you probably need to prefix unused vars with
  underscore in some patterns), but there are also some concerning
  function arguments.

- And there are some warnings about using cl functions at runtime.  I
  guess Emacs maintainers will want you to use only cl-lib?


HTH,

Michael.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]