GNU bug report logs - #35421
Also bind left image rotation key

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 35421 in the body.
You can then email your comments to 35421 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
To: bug-gnu-emacs <at> gnu.org
Subject: Also bind left image rotation key
Date: Thu, 25 Apr 2019 07:23:50 +0800

When viewing an image we type C-h m

   Image[imagemagick] mode defined in ‘image-mode.el’:

there is only one rotate command bound

   r		image-rotate

and it only rotates right. To rotate left one must hit it three times.

Also if we could see the whole image before rotation, its bottom is now
cut off after rotation. (landscape->portrait).

Message #8 received at 35421 <at> debbugs.gnu.org (full text, mbox):

From: 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 35421 <at> debbugs.gnu.org
Subject: Re: bug#35421: Also bind left image rotation key
Date: Tue, 09 Jul 2019 22:12:17 +0800

Can an adding an argument reverse the direction?

Message #11 received at 35421 <at> debbugs.gnu.org (full text, mbox):

From: 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 35421 <at> debbugs.gnu.org
Subject: Re: bug#35421: Also bind left image rotation key
Date: Tue, 09 Jul 2019 22:13:30 +0800

Or maybe [argument x 90 degrees].

Message #14 received at 35421 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
Cc: 35421 <at> debbugs.gnu.org
Subject: Re: bug#35421: Also bind left image rotation key
Date: Tue, 09 Jul 2019 16:04:49 +0200

積丹尼 Dan Jacobson <jidanni <at> jidanni.org> writes:

> When viewing an image we type C-h m
>
>    Image[imagemagick] mode defined in ‘image-mode.el’:
>
> there is only one rotate command bound
>
>    r		image-rotate
>
> and it only rotates right. To rotate left one must hit it three times.

I don't think it's worth adding more keystrokes to the image map --
images can appear in any mode and it's better to keep the number of
keystrokes limited.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #21 received at 35421 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 35421 <at> debbugs.gnu.org,
 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
Subject: Re: bug#35421: Also bind left image rotation key
Date: Tue, 09 Jul 2019 15:54:34 +0100

reopen 35421
tags 35421 - wontfix
quit

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> 積丹尼 Dan Jacobson <jidanni <at> jidanni.org> writes:
>
>> When viewing an image we type C-h m
>>
>>    Image[imagemagick] mode defined in ‘image-mode.el’:
>>
>> there is only one rotate command bound
>>
>>    r		image-rotate
>>
>> and it only rotates right. To rotate left one must hit it three times.
>
> I don't think it's worth adding more keystrokes to the image map --
> images can appear in any mode and it's better to keep the number of
> keystrokes limited.

I had a patch prepared for giving image-rotate an optional prefix
argument, but then I got distracted by some imagemagick vs native
transformation issues, so I was waiting for the native support to
stabilise a bit before returning to this in earnest.  Should have
something to show within a day or two.

Thanks,

-- 
Basil

Message #28 received at 35421 <at> debbugs.gnu.org (full text, mbox):

From: 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 35421 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>
Subject: Re: bug#35421: Also bind left image rotation key
Date: Tue, 09 Jul 2019 23:00:21 +0800

args
1,2,3=
90,180,270
or something like that.

Message #31 received at 35421 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 35421 <at> debbugs.gnu.org,
 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
Subject: Re: bug#35421: Also bind left image rotation key
Date: Wed, 17 Jul 2019 00:45:08 +0100

[Message part 1 (text/plain, inline)]

tags 35421 + patch
quit

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> I had a patch prepared for giving image-rotate an optional prefix
> argument, but then I got distracted by some imagemagick vs native
> transformation issues, so I was waiting for the native support to
> stabilise a bit before returning to this in earnest.  Should have
> something to show within a day or two.

With a slight delay due to a camping trip, here it is:

[0001-Allow-counter-clockwise-rotations-in-image-rotate.patch (text/x-diff, inline)]

From 38e676d04f12a69289d4095bb9f61247c4f08ae8 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Date: Tue, 16 Jul 2019 22:51:27 +0100
Subject: [PATCH] Allow counter-clockwise rotations in image-rotate

* lisp/image.el (image-rotate): Extend with an optional argument
specifying the rotation in degrees (bug#35421).
* doc/lispref/display.texi (Showing Images):
* etc/NEWS: Document the change.
* test/lisp/image-tests.el (image-rotate): New test.
---
 doc/lispref/display.texi |  3 ++-
 etc/NEWS                 |  5 +++++
 lisp/image.el            | 22 +++++++++++++---------
 test/lisp/image-tests.el | 23 +++++++++++++++++++++++
 4 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index a38569f726..4b10788862 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -5992,7 +5992,8 @@ Showing Images
 of @samp{4} means to decrease the size by 40%.  The default is 20%.
 
 @item r
-Rotate the image by 90 degrees (@code{image-rotate}).
+Rotate the image by 90 degrees clockwise (@code{image-rotate}).
+A prefix means to rotate by 90 degrees counter-clockwise instead.
 
 @item o
 Save the image to a file (@code{image-save}).
diff --git a/etc/NEWS b/etc/NEWS
index 76ea1df821..4cc30dfcbd 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2237,6 +2237,11 @@ The image parameters 'image-transform-rotation',
 buffer-local, so each buffer could have its own values for these
 parameters.
 
++++
+*** The command 'image-rotate' now accepts a prefix argument.
+With a prefix argument, 'image-rotate' now rotates the image at point
+90 degrees counter-clockwise, instead of the default clockwise.
+
 ** Modules
 
 *** The function 'load' now behaves correctly when loading modules.
diff --git a/lisp/image.el b/lisp/image.el
index b58b1dc954..c3e28655c3 100644
--- a/lisp/image.el
+++ b/lisp/image.el
@@ -1028,16 +1028,20 @@ image--current-scaling
         (display-width (car (image-size image t))))
     (/ (float display-width) image-width)))
 
-(defun image-rotate ()
-  "Rotate the image under point by 90 degrees clockwise."
-  (interactive)
+(defun image-rotate (&optional angle)
+  "Rotate the image under point by ANGLE degrees clockwise.
+If nil, ANGLE defaults to 90.  Interactively, rotate the image 90
+degrees clockwise with no prefix argument, and counter-clockwise
+with a prefix argument.  Note that most image types support
+rotations by only multiples of 90 degrees."
+  (interactive (and current-prefix-arg '(-90)))
   (let ((image (image--get-imagemagick-and-warn)))
-    (plist-put (cdr image) :rotation
-               (float (mod (+ (or (plist-get (cdr image) :rotation) 0) 90)
-                           ;; We don't want to exceed 360 degrees
-                           ;; rotation, because it's not seen as valid
-                           ;; in exif data.
-                           360)))))
+    (setf (image-property image :rotation)
+          (float (mod (+ (or (image-property image :rotation) 0)
+                         (or angle 90))
+                      ;; We don't want to exceed 360 degrees rotation,
+                      ;; because it's not seen as valid in Exif data.
+                      360)))))
 
 (defun image-save ()
   "Save the image under point."
diff --git a/test/lisp/image-tests.el b/test/lisp/image-tests.el
index 5a5b8ea1f7..01c81e3022 100644
--- a/test/lisp/image-tests.el
+++ b/test/lisp/image-tests.el
@@ -21,6 +21,8 @@
 
 (require 'ert)
 (require 'image)
+(eval-when-compile
+  (require 'cl-lib))
 
 (defconst image-tests--emacs-images-directory
   (expand-file-name "../etc/images" (getenv "EMACS_TEST_DIRECTORY"))
@@ -53,4 +55,25 @@ image-type-from-file-header-test
 	       (expand-file-name "splash.svg"
 				 image-tests--emacs-images-directory)))))
 
+(ert-deftest image-rotate ()
+  "Test `image-rotate'."
+  (cl-letf* ((image (list 'image))
+             ((symbol-function 'image--get-imagemagick-and-warn)
+              (lambda () image)))
+    (let ((current-prefix-arg '(4)))
+      (call-interactively #'image-rotate))
+    (should (equal image '(image :rotation 270.0)))
+    (call-interactively #'image-rotate)
+    (should (equal image '(image :rotation 0.0)))
+    (image-rotate)
+    (should (equal image '(image :rotation 90.0)))
+    (image-rotate 0)
+    (should (equal image '(image :rotation 90.0)))
+    (image-rotate 1)
+    (should (equal image '(image :rotation 91.0)))
+    (image-rotate 1234.5)
+    (should (equal image '(image :rotation 245.5)))
+    (image-rotate -154.5)
+    (should (equal image '(image :rotation 91.0)))))
+
 ;;; image-tests.el ends here
-- 
2.20.1

[Message part 3 (text/plain, inline)]

I think this is a good solution for several reasons:

1. It does not require a new key binding, and it does not overly
   complicate the calling convention of the current key binding.

2. It turns image-rotate into a general image-rotating subroutine,
   which users and library authors alike can reuse.

2.1. It does not limit rotations of imagemagick images to multiples of
     90 degrees.

and I don't see any drawbacks, so I would like to push this to master,
subject to comments/objections.

Thanks,

-- 
Basil

Message #36 received at 35421 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
Cc: 35421 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>
Subject: Re: bug#35421: Also bind left image rotation key
Date: Wed, 17 Jul 2019 00:52:59 +0100

積丹尼 Dan Jacobson <jidanni <at> jidanni.org> writes:

> args
> 1,2,3=
> 90,180,270
> or something like that.

Image viewers like gThumb provide buttons for rotations in steps of 90
degrees, and Emacs isn't even an image viewer, so there's no need to
complicate and restrict (w.r.t. future changes) the command's calling
convention like that.

Instead, the proposed patch[1] makes the simplest possible change to
support counter-clockwise rotations in one step, rather than three.  In
addition, it makes image-rotate accept arbitrary rotation angles when
called from Lisp, so you can use it to easily write your own
image-rotating command, with your preferred calling convention.

[1]: https://debbugs.gnu.org/35421#31

-- 
Basil

Message #39 received at 35421 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 35421 <at> debbugs.gnu.org,
 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
Subject: Re: bug#35421: Also bind left image rotation key
Date: Wed, 17 Jul 2019 13:03:40 +0200

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> I think this is a good solution for several reasons:
>
> 1. It does not require a new key binding, and it does not overly
>    complicate the calling convention of the current key binding.
>
> 2. It turns image-rotate into a general image-rotating subroutine,
>    which users and library authors alike can reuse.
>
> 2.1. It does not limit rotations of imagemagick images to multiples of
>      90 degrees.
>
> and I don't see any drawbacks, so I would like to push this to master,
> subject to comments/objections.

Looks good to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #42 received at 35421 <at> debbugs.gnu.org (full text, mbox):

From: 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 35421 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>
Subject: Re: bug#35421: Also bind left image rotation key
Date: Fri, 19 Jul 2019 08:53:16 +0800

OK but add to Docstring/Info:
To rotate (right) by 90, hit ...
To rotate (right) by 180, hit ...
To rotate (right) by 270, hit ...
As these are the most common tasks.

(I'm most curious as to the recommended sequences the users should hit,
especially for 180.

Message #45 received at 35421 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 35421 <at> debbugs.gnu.org,
 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
Subject: Re: bug#35421: Also bind left image rotation key
Date: Sat, 20 Jul 2019 16:06:44 +0100

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Looks good to me.

Thanks, pushed to master:

Allow counter-clockwise rotations in image-rotate
b728620a75 2019-07-20 16:00:31 +0100
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=b728620a756db78b8cb0a41afa72db6209102cdf

-- 
Basil

Message #52 received at 35421-done <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: 積丹尼 Dan Jacobson <jidanni <at> jidanni.org>
Cc: 35421-done <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>
Subject: Re: bug#35421: Also bind left image rotation key
Date: Sat, 20 Jul 2019 16:12:50 +0100

tags 35421 fixed
close 35421 27.1
quit

積丹尼 Dan Jacobson <jidanni <at> jidanni.org> writes:

> OK but add to Docstring/Info:
> To rotate (right) by 90, hit ...
> To rotate (right) by 180, hit ...
> To rotate (right) by 270, hit ...
> As these are the most common tasks.

That would be superfluous, as the documentation in the applied patch
already explains how to rotate by multiples of 90 degrees interactively,
and by an arbitrary angle from Lisp.

So I'm closing this report as done.

> (I'm most curious as to the recommended sequences the users should hit,
> especially for 180.

A rotation of 180 degrees is equivalent to two consecutive rotations of
90 degrees in either direction.

In vanilla Emacs that corresponds to 'r r' or 'C-u r C-u r'.
In custom Emacs it could be bound to any number of preferred keys.

Thanks,

-- 
Basil

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.