* image-dired: Check all external programs available
@ 2016-08-28 14:58 Tino Calancha
2016-08-30 12:11 ` Mathias Dahl
0 siblings, 1 reply; 5+ messages in thread
From: Tino Calancha @ 2016-08-28 14:58 UTC (permalink / raw)
To: Emacs developers; +Cc: tino.calancha
image-dired uses several external programs. The file
introduces user options with the name of the executables;
for instance, image-dired-cmd-create-thumbnail-program
has default value 'convert'.
If an user don't have installed one of these external programs,
and s?he run one function requiring that missing executable,
then the error is not very clear.
For instance, calling image-dired-rotate-original when
image-dired-cmd-rotate-original-program is not installed throw an error:
'Could not rotate image'.
For this case seems better to report that the required executable
is not available. Such error would avoid the need to read the code
to understand what was actually wrong.
I propose a patch which set default value nil for the option
if the executable is not found. Every function using an external
program has a nil check for the option storing the executable name:
when the option is nil, the function throw an error reporting that the
option is nil.
Do you think this proposal is useful?
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From 4cf0cb63adec2d5b6c89eee78645106d2479462f Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Sun, 28 Aug 2016 23:33:09 +0900
Subject: [PATCH] image-dired: Report when a necessary executable is not
found
* lisp/image-dired.el (image-dired-cmd-rotate-original-program)
(image-dired-cmd-create-thumbnail-program)
(image-dired-cmd-create-temp-image-program)
(image-dired-cmd-rotate-thumbnail-program)
(image-dired-cmd-write-exif-data-program)
(image-dired-cmd-read-exif-data-program):
Use executable-find to set the defaut value for this option.
(image-dired-cmd-rotate-original-program):
Search for program 'convert' if 'jpegtran' is not found.
(image-dired-cmd-rotate-original-options):
Set the default value consistent with the executable in
image-dired-cmd-rotate-original-program.
(image-dired-thumb-name, image-dired-display-image)
(image-dired-image-at-point-p, image-dired-rotate-original)
(image-dired-thumbnail-set-image-description)
(image-dired-set-exif-data):
Throw and error when the executable used in the function is not
found.
---
lisp/image-dired.el | 110
++++++++++++++++++++++++++++++----------------------
1 file changed, 64 insertions(+), 46 deletions(-)
diff --git a/lisp/image-dired.el b/lisp/image-dired.el
index 67b023d..2511e13 100644
--- a/lisp/image-dired.el
+++ b/lisp/image-dired.el
@@ -224,7 +224,7 @@ image-dired-gallery-thumb-image-root-url
:group 'image-dired)
(defcustom image-dired-cmd-create-thumbnail-program
- "convert"
+ (executable-find "convert")
"Executable used to create thumbnail.
Used together with `image-dired-cmd-create-thumbnail-options'."
:type 'string
@@ -242,7 +242,7 @@ image-dired-cmd-create-thumbnail-options
:group 'image-dired)
(defcustom image-dired-cmd-create-temp-image-program
- "convert"
+ (executable-find "convert")
"Executable used to create temporary image.
Used together with `image-dired-cmd-create-temp-image-options'."
:type 'string
@@ -308,7 +308,7 @@ image-dired-cmd-create-standard-thumbnail-command
:group 'image-dired)
(defcustom image-dired-cmd-rotate-thumbnail-program
- "mogrify"
+ (executable-find "mogrify")
"Executable used to rotate thumbnail.
Used together with `image-dired-cmd-rotate-thumbnail-options'."
:type 'string
@@ -326,14 +326,20 @@ image-dired-cmd-rotate-thumbnail-options
:group 'image-dired)
(defcustom image-dired-cmd-rotate-original-program
- "jpegtran"
+ (cond ((executable-find "jpegtran"))
+ ((executable-find "convert")))
"Executable used to rotate original image.
Used together with `image-dired-cmd-rotate-original-options'."
:type 'string
:group 'image-dired)
(defcustom image-dired-cmd-rotate-original-options
- "%p -rotate %d -copy all -outfile %t \"%o\""
+ (when image-dired-cmd-rotate-original-program
+ (pcase image-dired-cmd-rotate-original-program
+ ((pred (lambda (x) (string-match "jpegtran" x)))
+ "%p -rotate %d -copy all -outfile %t \"%o\"")
+ ((pred (lambda (x) (string-match "convert" x)))
+ "%p -rotate %d \"%o\" %t")))
"Format of command used to rotate original image.
Available options are %p which is replaced by
`image-dired-cmd-rotate-original-program', %d which is replaced by the
@@ -358,7 +364,7 @@ image-dired-rotate-original-ask-before-overwrite
:group 'image-dired)
(defcustom image-dired-cmd-write-exif-data-program
- "exiftool"
+ (executable-find "exiftool")
"Program used to write EXIF data to image.
Used together with `image-dired-cmd-write-exif-data-options'."
:type 'string
@@ -375,7 +381,7 @@ image-dired-cmd-write-exif-data-options
:group 'image-dired)
(defcustom image-dired-cmd-read-exif-data-program
- "exiftool"
+ (executable-find "exiftool")
"Program used to read EXIF data to image.
Used together with `image-dired-cmd-read-exif-data-program-options'."
:type 'string
@@ -615,6 +621,8 @@ image-dired-thumb-name
(defun image-dired-create-thumb (original-file thumbnail-file)
"For ORIGINAL-FILE, create thumbnail image named THUMBNAIL-FILE."
+ (unless image-dired-cmd-create-thumbnail-program
+ (error "image-dired-cmd-create-thumbnail-program is nil"))
(let* ((width (int-to-string image-dired-thumb-width))
(height (int-to-string image-dired-thumb-height))
(modif-time (format "%.0f" (float-time (nth 5 (file-attributes
@@ -1810,6 +1818,8 @@ image-dired-display-image
(progn
(setq width (image-dired-display-window-width))
(setq height (image-dired-display-window-height))
+ (unless image-dired-cmd-create-temp-image-program
+ (error "image-dired-cmd-create-temp-image-program is nil"))
(setq command
(format-spec
image-dired-cmd-create-temp-image-options
@@ -1866,20 +1876,22 @@ image-dired-image-at-point-p
(defun image-dired-rotate-thumbnail (degrees)
"Rotate thumbnail DEGREES degrees."
- (if (not (image-dired-image-at-point-p))
- (message "No thumbnail at point")
- (let ((file (image-dired-thumb-name
(image-dired-original-file-name)))
- command)
- (setq command (format-spec
- image-dired-cmd-rotate-thumbnail-options
- (list
- (cons ?p image-dired-cmd-rotate-thumbnail-program)
- (cons ?d degrees)
- (cons ?t (expand-file-name file)))))
- (call-process shell-file-name nil nil nil shell-command-switch
command)
- ;; Clear the cache to refresh image. I wish I could just refresh
- ;; the current file but I do not know how to do that. Yet...
- (clear-image-cache))))
+ (unless (image-dired-image-at-point-p)
+ (message "No thumbnail at point"))
+ (unless image-dired-cmd-rotate-thumbnail-program
+ (error "image-dired-cmd-rotate-thumbnail-program is nil"))
+ (let ((file (image-dired-thumb-name (image-dired-original-file-name)))
+ command)
+ (setq command (format-spec
+ image-dired-cmd-rotate-thumbnail-options
+ (list
+ (cons ?p image-dired-cmd-rotate-thumbnail-program)
+ (cons ?d degrees)
+ (cons ?t (expand-file-name file)))))
+ (call-process shell-file-name nil nil nil shell-command-switch
command)
+ ;; Clear the cache to refresh image. I wish I could just refresh
+ ;; the current file but I do not know how to do that. Yet...
+ (clear-image-cache)))
(defun image-dired-rotate-thumbnail-left ()
"Rotate thumbnail left (counter clockwise) 90 degrees.
@@ -1908,31 +1920,33 @@ image-dired-refresh-thumb
(defun image-dired-rotate-original (degrees)
"Rotate original image DEGREES degrees."
- (if (not (image-dired-image-at-point-p))
- (message "No image at point")
- (let ((file (image-dired-original-file-name))
- command)
- (if (not (string-match "\\.[jJ][pP[eE]?[gG]$" file))
- (error "Only JPEG images can be rotated!"))
- (setq command (format-spec
- image-dired-cmd-rotate-original-options
- (list
- (cons ?p image-dired-cmd-rotate-original-program)
- (cons ?d degrees)
- (cons ?o (expand-file-name file))
- (cons ?t image-dired-temp-rotate-image-file))))
- (if (not (= 0 (call-process shell-file-name nil nil nil
- shell-command-switch command)))
- (error "Could not rotate image")
- (image-dired-display-image image-dired-temp-rotate-image-file)
- (if (or (and image-dired-rotate-original-ask-before-overwrite
- (y-or-n-p
- "Rotate to temp file OK. Overwrite original image?
"))
- (not image-dired-rotate-original-ask-before-overwrite))
- (progn
- (copy-file image-dired-temp-rotate-image-file file t)
- (image-dired-refresh-thumb))
- (image-dired-display-image file))))))
+ (unless (image-dired-image-at-point-p)
+ (message "No image at point"))
+ (unless image-dired-cmd-rotate-original-program
+ (error "image-dired-cmd-rotate-original-program is nil"))
+ (let ((file (image-dired-original-file-name))
+ command)
+ (if (not (string-match "\\.[jJ][pP[eE]?[gG]$" file))
+ (error "Only JPEG images can be rotated!"))
+ (setq command (format-spec
+ image-dired-cmd-rotate-original-options
+ (list
+ (cons ?p image-dired-cmd-rotate-original-program)
+ (cons ?d degrees)
+ (cons ?o (expand-file-name file))
+ (cons ?t image-dired-temp-rotate-image-file))))
+ (if (not (= 0 (call-process shell-file-name nil nil nil
+ shell-command-switch command)))
+ (error "Could not rotate image")
+ (image-dired-display-image image-dired-temp-rotate-image-file)
+ (if (or (and image-dired-rotate-original-ask-before-overwrite
+ (y-or-n-p
+ "Rotate to temp file OK. Overwrite original image?
"))
+ (not image-dired-rotate-original-ask-before-overwrite))
+ (progn
+ (copy-file image-dired-temp-rotate-image-file file t)
+ (image-dired-refresh-thumb))
+ (image-dired-display-image file)))))
(defun image-dired-rotate-original-left ()
"Rotate original image left (counter clockwise) 90 degrees."
@@ -1987,6 +2001,8 @@ image-dired-thumbnail-set-image-description
(defun image-dired-set-exif-data (file tag-name tag-value)
"In FILE, set EXIF tag TAG-NAME to value TAG-VALUE."
+ (unless image-dired-cmd-write-exif-data-program
+ (error "image-dired-cmd-write-exif-data-program is nil"))
(let (command)
(setq command (format-spec
image-dired-cmd-write-exif-data-options
@@ -1999,6 +2015,8 @@ image-dired-set-exif-data
(defun image-dired-get-exif-data (file tag-name)
"From FILE, return EXIF tag TAG-NAME."
+ (unless image-dired-cmd-read-exif-data-program
+ (error "image-dired-cmd-read-exif-data-program is nil"))
(let ((buf (get-buffer-create "*image-dired-get-exif-data*"))
command tag-value)
(setq command (format-spec
--
2.9.3
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.20.9)
of 2016-08-28
Repository revision: 7fcce24e75b8281621a0b8816dc58cbdc05fdc91
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: image-dired: Check all external programs available
2016-08-28 14:58 image-dired: Check all external programs available Tino Calancha
@ 2016-08-30 12:11 ` Mathias Dahl
2016-08-31 12:07 ` Tino Calancha
0 siblings, 1 reply; 5+ messages in thread
From: Mathias Dahl @ 2016-08-30 12:11 UTC (permalink / raw)
To: Tino Calancha; +Cc: Emacs developers
[-- Attachment #1: Type: text/plain, Size: 12954 bytes --]
Hi Tino,
Thanks for the proposal and the fix!
I'm the long lost author of image-dired.el :)
From my viewpoint, and after just a quick overview of the changes, I think
it looks good. If we can present more meaningful error messages to the
user, it can only be good. I noticed you did a small change in the logic
surrounding jpegtrans; IIUC you fallback to use "convert" if jpegtrans is
not available. As long as you have tested both scenarios I'm fine with the
change. The same goes for the other changes of course, that they are tested
and works well.
Someone else has to apply this to the Emacs source code however, since I'm
not up-to-date with how commits are done these days.
Thanks!
/Mathias
On Sun, Aug 28, 2016 at 4:58 PM, Tino Calancha <tino.calancha@gmail.com>
wrote:
>
> image-dired uses several external programs. The file
> introduces user options with the name of the executables;
> for instance, image-dired-cmd-create-thumbnail-program
> has default value 'convert'.
>
> If an user don't have installed one of these external programs,
> and s?he run one function requiring that missing executable,
> then the error is not very clear.
>
> For instance, calling image-dired-rotate-original when
> image-dired-cmd-rotate-original-program is not installed throw an error:
> 'Could not rotate image'.
> For this case seems better to report that the required executable
> is not available. Such error would avoid the need to read the code
> to understand what was actually wrong.
>
> I propose a patch which set default value nil for the option
> if the executable is not found. Every function using an external
> program has a nil check for the option storing the executable name:
> when the option is nil, the function throw an error reporting that the
> option is nil.
>
> Do you think this proposal is useful?
>
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> ;;;;;;;;;;;;;;;;;;;
> From 4cf0cb63adec2d5b6c89eee78645106d2479462f Mon Sep 17 00:00:00 2001
> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Sun, 28 Aug 2016 23:33:09 +0900
> Subject: [PATCH] image-dired: Report when a necessary executable is not
> found
>
> * lisp/image-dired.el (image-dired-cmd-rotate-original-program)
> (image-dired-cmd-create-thumbnail-program)
> (image-dired-cmd-create-temp-image-program)
> (image-dired-cmd-rotate-thumbnail-program)
> (image-dired-cmd-write-exif-data-program)
> (image-dired-cmd-read-exif-data-program):
> Use executable-find to set the defaut value for this option.
> (image-dired-cmd-rotate-original-program):
> Search for program 'convert' if 'jpegtran' is not found.
> (image-dired-cmd-rotate-original-options):
> Set the default value consistent with the executable in
> image-dired-cmd-rotate-original-program.
> (image-dired-thumb-name, image-dired-display-image)
> (image-dired-image-at-point-p, image-dired-rotate-original)
> (image-dired-thumbnail-set-image-description)
> (image-dired-set-exif-data):
> Throw and error when the executable used in the function is not
> found.
> ---
> lisp/image-dired.el | 110 ++++++++++++++++++++++++++++++
> ----------------------
> 1 file changed, 64 insertions(+), 46 deletions(-)
>
> diff --git a/lisp/image-dired.el b/lisp/image-dired.el
> index 67b023d..2511e13 100644
> --- a/lisp/image-dired.el
> +++ b/lisp/image-dired.el
> @@ -224,7 +224,7 @@ image-dired-gallery-thumb-image-root-url
> :group 'image-dired)
>
> (defcustom image-dired-cmd-create-thumbnail-program
> - "convert"
> + (executable-find "convert")
> "Executable used to create thumbnail.
> Used together with `image-dired-cmd-create-thumbnail-options'."
> :type 'string
> @@ -242,7 +242,7 @@ image-dired-cmd-create-thumbnail-options
> :group 'image-dired)
>
> (defcustom image-dired-cmd-create-temp-image-program
> - "convert"
> + (executable-find "convert")
> "Executable used to create temporary image.
> Used together with `image-dired-cmd-create-temp-image-options'."
> :type 'string
> @@ -308,7 +308,7 @@ image-dired-cmd-create-standard-thumbnail-command
> :group 'image-dired)
>
> (defcustom image-dired-cmd-rotate-thumbnail-program
> - "mogrify"
> + (executable-find "mogrify")
> "Executable used to rotate thumbnail.
> Used together with `image-dired-cmd-rotate-thumbnail-options'."
> :type 'string
> @@ -326,14 +326,20 @@ image-dired-cmd-rotate-thumbnail-options
> :group 'image-dired)
>
> (defcustom image-dired-cmd-rotate-original-program
> - "jpegtran"
> + (cond ((executable-find "jpegtran"))
> + ((executable-find "convert")))
> "Executable used to rotate original image.
> Used together with `image-dired-cmd-rotate-original-options'."
> :type 'string
> :group 'image-dired)
>
> (defcustom image-dired-cmd-rotate-original-options
> - "%p -rotate %d -copy all -outfile %t \"%o\""
> + (when image-dired-cmd-rotate-original-program
> + (pcase image-dired-cmd-rotate-original-program
> + ((pred (lambda (x) (string-match "jpegtran" x)))
> + "%p -rotate %d -copy all -outfile %t \"%o\"")
> + ((pred (lambda (x) (string-match "convert" x)))
> + "%p -rotate %d \"%o\" %t")))
> "Format of command used to rotate original image.
> Available options are %p which is replaced by
> `image-dired-cmd-rotate-original-program', %d which is replaced by the
> @@ -358,7 +364,7 @@ image-dired-rotate-original-ask-before-overwrite
> :group 'image-dired)
>
> (defcustom image-dired-cmd-write-exif-data-program
> - "exiftool"
> + (executable-find "exiftool")
> "Program used to write EXIF data to image.
> Used together with `image-dired-cmd-write-exif-data-options'."
> :type 'string
> @@ -375,7 +381,7 @@ image-dired-cmd-write-exif-data-options
> :group 'image-dired)
>
> (defcustom image-dired-cmd-read-exif-data-program
> - "exiftool"
> + (executable-find "exiftool")
> "Program used to read EXIF data to image.
> Used together with `image-dired-cmd-read-exif-data-program-options'."
> :type 'string
> @@ -615,6 +621,8 @@ image-dired-thumb-name
>
> (defun image-dired-create-thumb (original-file thumbnail-file)
> "For ORIGINAL-FILE, create thumbnail image named THUMBNAIL-FILE."
> + (unless image-dired-cmd-create-thumbnail-program
> + (error "image-dired-cmd-create-thumbnail-program is nil"))
> (let* ((width (int-to-string image-dired-thumb-width))
> (height (int-to-string image-dired-thumb-height))
> (modif-time (format "%.0f" (float-time (nth 5 (file-attributes
> @@ -1810,6 +1818,8 @@ image-dired-display-image
> (progn
> (setq width (image-dired-display-window-width))
> (setq height (image-dired-display-window-height))
> + (unless image-dired-cmd-create-temp-image-program
> + (error "image-dired-cmd-create-temp-image-program is nil"))
> (setq command
> (format-spec
> image-dired-cmd-create-temp-image-options
> @@ -1866,20 +1876,22 @@ image-dired-image-at-point-p
>
> (defun image-dired-rotate-thumbnail (degrees)
> "Rotate thumbnail DEGREES degrees."
> - (if (not (image-dired-image-at-point-p))
> - (message "No thumbnail at point")
> - (let ((file (image-dired-thumb-name (image-dired-original-file-nam
> e)))
> - command)
> - (setq command (format-spec
> - image-dired-cmd-rotate-thumbnail-options
> - (list
> - (cons ?p image-dired-cmd-rotate-thumbnail-program)
> - (cons ?d degrees)
> - (cons ?t (expand-file-name file)))))
> - (call-process shell-file-name nil nil nil shell-command-switch
> command)
> - ;; Clear the cache to refresh image. I wish I could just refresh
> - ;; the current file but I do not know how to do that. Yet...
> - (clear-image-cache))))
> + (unless (image-dired-image-at-point-p)
> + (message "No thumbnail at point"))
> + (unless image-dired-cmd-rotate-thumbnail-program
> + (error "image-dired-cmd-rotate-thumbnail-program is nil"))
> + (let ((file (image-dired-thumb-name (image-dired-original-file-name)))
> + command)
> + (setq command (format-spec
> + image-dired-cmd-rotate-thumbnail-options
> + (list
> + (cons ?p image-dired-cmd-rotate-thumbnail-program)
> + (cons ?d degrees)
> + (cons ?t (expand-file-name file)))))
> + (call-process shell-file-name nil nil nil shell-command-switch
> command)
> + ;; Clear the cache to refresh image. I wish I could just refresh
> + ;; the current file but I do not know how to do that. Yet...
> + (clear-image-cache)))
>
> (defun image-dired-rotate-thumbnail-left ()
> "Rotate thumbnail left (counter clockwise) 90 degrees.
> @@ -1908,31 +1920,33 @@ image-dired-refresh-thumb
>
> (defun image-dired-rotate-original (degrees)
> "Rotate original image DEGREES degrees."
> - (if (not (image-dired-image-at-point-p))
> - (message "No image at point")
> - (let ((file (image-dired-original-file-name))
> - command)
> - (if (not (string-match "\\.[jJ][pP[eE]?[gG]$" file))
> - (error "Only JPEG images can be rotated!"))
> - (setq command (format-spec
> - image-dired-cmd-rotate-original-options
> - (list
> - (cons ?p image-dired-cmd-rotate-original-program)
> - (cons ?d degrees)
> - (cons ?o (expand-file-name file))
> - (cons ?t image-dired-temp-rotate-image-file))))
> - (if (not (= 0 (call-process shell-file-name nil nil nil
> - shell-command-switch command)))
> - (error "Could not rotate image")
> - (image-dired-display-image image-dired-temp-rotate-image-file)
> - (if (or (and image-dired-rotate-original-ask-before-overwrite
> - (y-or-n-p
> - "Rotate to temp file OK. Overwrite original image?
> "))
> - (not image-dired-rotate-original-ask-before-overwrite))
> - (progn
> - (copy-file image-dired-temp-rotate-image-file file t)
> - (image-dired-refresh-thumb))
> - (image-dired-display-image file))))))
> + (unless (image-dired-image-at-point-p)
> + (message "No image at point"))
> + (unless image-dired-cmd-rotate-original-program
> + (error "image-dired-cmd-rotate-original-program is nil"))
> + (let ((file (image-dired-original-file-name))
> + command)
> + (if (not (string-match "\\.[jJ][pP[eE]?[gG]$" file))
> + (error "Only JPEG images can be rotated!"))
> + (setq command (format-spec
> + image-dired-cmd-rotate-original-options
> + (list
> + (cons ?p image-dired-cmd-rotate-original-program)
> + (cons ?d degrees)
> + (cons ?o (expand-file-name file))
> + (cons ?t image-dired-temp-rotate-image-file))))
> + (if (not (= 0 (call-process shell-file-name nil nil nil
> + shell-command-switch command)))
> + (error "Could not rotate image")
> + (image-dired-display-image image-dired-temp-rotate-image-file)
> + (if (or (and image-dired-rotate-original-ask-before-overwrite
> + (y-or-n-p
> + "Rotate to temp file OK. Overwrite original image?
> "))
> + (not image-dired-rotate-original-ask-before-overwrite))
> + (progn
> + (copy-file image-dired-temp-rotate-image-file file t)
> + (image-dired-refresh-thumb))
> + (image-dired-display-image file)))))
>
> (defun image-dired-rotate-original-left ()
> "Rotate original image left (counter clockwise) 90 degrees."
> @@ -1987,6 +2001,8 @@ image-dired-thumbnail-set-image-description
>
> (defun image-dired-set-exif-data (file tag-name tag-value)
> "In FILE, set EXIF tag TAG-NAME to value TAG-VALUE."
> + (unless image-dired-cmd-write-exif-data-program
> + (error "image-dired-cmd-write-exif-data-program is nil"))
> (let (command)
> (setq command (format-spec
> image-dired-cmd-write-exif-data-options
> @@ -1999,6 +2015,8 @@ image-dired-set-exif-data
>
> (defun image-dired-get-exif-data (file tag-name)
> "From FILE, return EXIF tag TAG-NAME."
> + (unless image-dired-cmd-read-exif-data-program
> + (error "image-dired-cmd-read-exif-data-program is nil"))
> (let ((buf (get-buffer-create "*image-dired-get-exif-data*"))
> command tag-value)
> (setq command (format-spec
> --
> 2.9.3
>
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> ;;;;;;;;;;;;;;;;;;;
>
>
> In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.20.9)
> of 2016-08-28
> Repository revision: 7fcce24e75b8281621a0b8816dc58cbdc05fdc91
>
>
>
[-- Attachment #2: Type: text/html, Size: 16130 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: image-dired: Check all external programs available
2016-08-30 12:11 ` Mathias Dahl
@ 2016-08-31 12:07 ` Tino Calancha
2016-08-31 13:31 ` Mathias Dahl
0 siblings, 1 reply; 5+ messages in thread
From: Tino Calancha @ 2016-08-31 12:07 UTC (permalink / raw)
To: Mathias Dahl; +Cc: Emacs developers, Tino Calancha
On Tue, 30 Aug 2016, Mathias Dahl wrote:
>From my viewpoint, and after just a quick overview of the changes, I
> think it looks good. If we can present more meaningful error messages
>to the user, it can only be good.
>I noticed you did a small change in the logic surrounding jpegtran; IIUC
> you fallback to use "convert" if jpegtran is not available.
As you know, 'convert' also can rotate an image. I have noticed that
'jpegtran' is not included in the Debian repositories; 'convert' is
in Debian rep. and is also used to create the thumbnail and the temporary
image so it should be installed to use this lib anyway.
The default value for 'image-dired-cmd-rotate-original-options' is set
accordingly with the executable used: jpegtran or convert.
Another way could be to drop 'jpegtran' dependency and
just set 'convert' as default value for the following three options;
image-dired-cmd-create-thumbnail-program
image-dired-cmd-create-temp-image-program
image-dired-cmd-rotate-original-program
> As long as you have tested both scenarios I'm fine with the change.
> The same goes for the other changes of course, that they are tested and
>works well.
>Someone else has to apply this to the Emacs source code however, since
>I'm not up-to-date
>with how commits are done these days.
I will keep testing during this week. Then, if everything is OK
i will push the patch to the Emacs repository.
>Thanks!
Thank you very much!
Tino
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: image-dired: Check all external programs available
2016-08-31 12:07 ` Tino Calancha
@ 2016-08-31 13:31 ` Mathias Dahl
2016-09-04 13:49 ` Tino Calancha
0 siblings, 1 reply; 5+ messages in thread
From: Mathias Dahl @ 2016-08-31 13:31 UTC (permalink / raw)
To: Tino Calancha; +Cc: Emacs developers
[-- Attachment #1: Type: text/plain, Size: 629 bytes --]
>
> As you know, 'convert' also can rotate an image.
>
Yes, it probably can, but as I remember it, the rotation is lossy, i.e. it
makes the JPEG data worse than before the rotation. jpegtran was able to do
lossless rotation, which is why I picked it. For thumbnails it would not
matter but for rotating original files it's not nice to loose
information/quality. As long as users can get hold of jpegtran I think it
should be the default program used for rotation.
I will keep testing during this week. Then, if everything is OK
> i will push the patch to the Emacs repository.
>
Sounds good, thanks for doing this!
/Mathias
[-- Attachment #2: Type: text/html, Size: 1061 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: image-dired: Check all external programs available
2016-08-31 13:31 ` Mathias Dahl
@ 2016-09-04 13:49 ` Tino Calancha
0 siblings, 0 replies; 5+ messages in thread
From: Tino Calancha @ 2016-09-04 13:49 UTC (permalink / raw)
To: Mathias Dahl; +Cc: Emacs developers, Tino Calancha
On Wed, 31 Aug 2016, Mathias Dahl wrote:
> As you know, 'convert' also can rotate an image.
>
>
> Yes, it probably can, but as I remember it, the rotation is lossy, i.e. it makes the JPEG data worse than before the rotation.
> jpegtran was able to do lossless rotation, which is why I picked it. For thumbnails it would not matter but for rotating original
> files it's not nice to loose information/quality. As long as users can get hold of jpegtran I think it should be the default
> program used for rotation.
Thank you very much for the explanation.
I have pushed the changes to the master branch as commit ca473907
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-09-04 13:49 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-28 14:58 image-dired: Check all external programs available Tino Calancha
2016-08-30 12:11 ` Mathias Dahl
2016-08-31 12:07 ` Tino Calancha
2016-08-31 13:31 ` Mathias Dahl
2016-09-04 13:49 ` Tino Calancha
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).