* Re: master ca47390: image-dired: Report when a necessary executable is not found
2016-09-05 4:42 ` Clément Pit--Claudel
@ 2016-09-05 5:30 ` Tino Calancha
2016-09-05 6:41 ` Andreas Schwab
2016-09-05 15:48 ` Clément Pit--Claudel
2016-09-05 18:00 ` Tino Calancha
1 sibling, 2 replies; 10+ messages in thread
From: Tino Calancha @ 2016-09-05 5:30 UTC (permalink / raw)
To: Clément Pit--Claudel; +Cc: Emacs developers
[-- Attachment #1: Type: text/plain, Size: 5803 bytes --]
On Mon, 5 Sep 2016, Clément Pit--Claudel wrote:
> On 2016-09-04 23:42, Tino Calancha wrote:
>> Please let me know if the following patch is OK:
>> …
>
>> (defcustom image-dired-cmd-create-thumbnail-program
>> - "convert"
>> + (executable-find "convert")
>
>> …
>
>> (defcustom image-dired-cmd-create-thumbnail-program
>> - (executable-find "convert")
>> + (when (executable-find "convert") "convert")
>
> I don't understand how these two consecutive patches works.
My motivation is to report more clearly when a necessary
executable is missing. If i don't have installed "jpegtran"
(image-dired-cmd-rotate-original-program) and i call
image-dired-rotate-original
i get the message: "Could not rotate image"
>Assume I don't have "convert". Before the patches Emacs would complain,
>and installing ImageMagick would fix the complaint.
>After, Emacs will complain about not finding "convert", but after I
>install ImageMagick Emacs will keep complaining, because the defcustom
>will still be nil. Is this intended? Or am I misunderstanding something?
Good point. I agree is a nuisance having to set those values
after installing the executable.
How about following more simple patch?:
It just check for the required executable at the top of each function
using it; it signals an error if the executable is not found:
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From cefb3ca722ade34f81f54b0f1a09b8ba759b1f69 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Mon, 5 Sep 2016 14:23:15 +0900
Subject: [PATCH] image-dired: Signal an error before calling a missing
executable
lisp/image-dired.el (image-dired-display-image)
(image-dired-rotate-thumbnail)
(image-dired-rotate-original)
(image-dired-set-exif-data)
(image-dired-get-exif-data):
Throw and error when the executable used in the function is missing.
(image-dired-next-line, image-dired-previous-line):
Use 'forward-line'.
---
lisp/image-dired.el | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/lisp/image-dired.el b/lisp/image-dired.el
index 34e4eae..617914a 100644
--- a/lisp/image-dired.el
+++ b/lisp/image-dired.el
@@ -615,6 +615,8 @@ image-dired-thumb-name
(defun image-dired-create-thumb (original-file thumbnail-file)
"For ORIGINAL-FILE, create thumbnail image named THUMBNAIL-FILE."
+ (unless (executable-find image-dired-cmd-create-thumbnail-program)
+ (error "Executable image-dired-cmd-create-thumbnail-program is not
found"))
(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
@@ -1144,7 +1146,8 @@ image-dired-next-line
"Move to next line and display properties."
(interactive)
(let ((goal-column (current-column)))
- (next-line))
+ (forward-line 1)
+ (move-to-column goal-column))
;; If we end up in an empty spot, back up to the next thumbnail.
(if (not (image-dired-image-at-point-p))
(image-dired-backward-image))
@@ -1157,7 +1160,8 @@ image-dired-previous-line
"Move to previous line and display properties."
(interactive)
(let ((goal-column (current-column)))
- (previous-line))
+ (forward-line -1)
+ (move-to-column goal-column))
;; If we end up in an empty spot, back up to the next
;; thumbnail. This should only happen if the user deleted a
;; thumbnail and did not refresh, so it is not very common. But we
@@ -1802,6 +1806,8 @@ image-dired-display-image
If optional argument ORIGINAL-SIZE is non-nil, display image in its
original size."
+ (unless (executable-find image-dired-cmd-create-temp-image-program)
+ (error "image-dired-cmd-create-temp-image-program is not found"))
(let ((new-file (expand-file-name image-dired-temp-image-file))
width height command ret
(image-type 'jpeg))
@@ -1866,6 +1872,8 @@ image-dired-image-at-point-p
(defun image-dired-rotate-thumbnail (degrees)
"Rotate thumbnail DEGREES degrees."
+ (unless (executable-find image-dired-cmd-rotate-thumbnail-program)
+ (error "image-dired-cmd-rotate-thumbnail-program is not found"))
(if (not (image-dired-image-at-point-p))
(message "No thumbnail at point")
(let ((file (image-dired-thumb-name
(image-dired-original-file-name)))
@@ -1908,6 +1916,8 @@ image-dired-refresh-thumb
(defun image-dired-rotate-original (degrees)
"Rotate original image DEGREES degrees."
+ (unless (executable-find image-dired-cmd-rotate-original-program)
+ (error "image-dired-cmd-rotate-original-program is not found"))
(if (not (image-dired-image-at-point-p))
(message "No image at point")
(let ((file (image-dired-original-file-name))
@@ -1986,6 +1996,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 (executable-findimage-dired-cmd-write-exif-data-program)
+ (error "image-dired-cmd-write-exif-data-program is not found"))
(let (command)
(setq command (format-spec
image-dired-cmd-write-exif-data-options
@@ -1998,6 +2010,8 @@ image-dired-set-exif-data
(defun image-dired-get-exif-data (file tag-name)
"From FILE, return EXIF tag TAG-NAME."
+ (unless (executable-find image-dired-cmd-read-exif-data-program)
+ (error "image-dired-cmd-read-exif-data-program is not found"))
(let ((buf (get-buffer-create "*image-dired-get-exif-data*"))
command tag-value)
(setq command (format-spec
--
2.9.3
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
Repository revision: 2db3307e8a966a8f652a210d8f8eb83daddd7d9f
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: master ca47390: image-dired: Report when a necessary executable is not found
2016-09-05 4:42 ` Clément Pit--Claudel
2016-09-05 5:30 ` Tino Calancha
@ 2016-09-05 18:00 ` Tino Calancha
1 sibling, 0 replies; 10+ messages in thread
From: Tino Calancha @ 2016-09-05 18:00 UTC (permalink / raw)
To: Clément Pit--Claudel; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 7415 bytes --]
On Mon, 5 Sep 2016, Clément Pit--Claudel wrote:
>Looks good, but maybe you could extract this to a separate function that
>takes the variable name and produces the error? There's a bit of
> duplication in the patch you posted.
Thanks for your help.
Sure, how about this new patch?
It reverts ca473907, so that the options storing the executable names are
never nil, i.e., there is no need to reset the options after installing
the missing executables.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From 37a295e7149b459ea1cccfbf0ce49c5a50676a52 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Tue, 6 Sep 2016 02:50:56 +0900
Subject: [PATCH] image-dired: Signal an error before calling a missing
executable
Reverts commit ca473907
* lisp/image-dired.el (image-dired--check-executable-exists): New defun.
Throw and error when the executable arg is missing.
(image-dired-display-image, image-dired-rotate-thumbnail)
(image-dired-rotate-original, image-dired-set-exif-data)
(image-dired-get-exif-data):
Use it.
---
lisp/image-dired.el | 48 +++++++++++++++++++++++-------------------------
1 file changed, 23 insertions(+), 25 deletions(-)
diff --git a/lisp/image-dired.el b/lisp/image-dired.el
index 5ac4600..07f66e0 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
- (executable-find "convert")
+ "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
- (executable-find "convert")
+ "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
- (executable-find "mogrify")
+ "mogrify"
"Executable used to rotate thumbnail.
Used together with `image-dired-cmd-rotate-thumbnail-options'."
:type 'string
@@ -326,20 +326,14 @@ image-dired-cmd-rotate-thumbnail-options
:group 'image-dired)
(defcustom image-dired-cmd-rotate-original-program
- (cond ((executable-find "jpegtran"))
- ((executable-find "convert")))
+ "jpegtran"
"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
- (when image-dired-cmd-rotate-original-program
- (pcase image-dired-cmd-rotate-original-program
- ((pred (lambda (x) (string-match-p "jpegtran" x)))
- "%p -rotate %d -copy all -outfile %t \"%o\"")
- ((pred (lambda (x) (string-match-p "convert" x)))
- "%p -rotate %d \"%o\" %t")))
+ "%p -rotate %d -copy all -outfile %t \"%o\""
"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
@@ -364,7 +358,7 @@ image-dired-rotate-original-ask-before-overwrite
:group 'image-dired)
(defcustom image-dired-cmd-write-exif-data-program
- (executable-find "exiftool")
+ "exiftool"
"Program used to write EXIF data to image.
Used together with `image-dired-cmd-write-exif-data-options'."
:type 'string
@@ -381,7 +375,7 @@ image-dired-cmd-write-exif-data-options
:group 'image-dired)
(defcustom image-dired-cmd-read-exif-data-program
- (executable-find "exiftool")
+ "exiftool"
"Program used to read EXIF data to image.
Used together with `image-dired-cmd-read-exif-data-program-options'."
:type 'string
@@ -619,10 +613,14 @@ image-dired-thumb-name
(file-name-base f)
(file-name-extension f))))))
+(defun image-dired--check-executable-exists (executable)
+ (unless (executable-find (symbol-value executable))
+ (error "Executable %S not found" executable)))
+
(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"))
+ (image-dired--check-executable-exists
+ 'image-dired-cmd-create-thumbnail-program)
(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
@@ -1820,8 +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"))
+ (image-dired--check-executable-exists
+ 'image-dired-cmd-create-temp-image-program)
(setq command
(format-spec
image-dired-cmd-create-temp-image-options
@@ -1878,8 +1876,8 @@ image-dired-image-at-point-p
(defun image-dired-rotate-thumbnail (degrees)
"Rotate thumbnail DEGREES degrees."
- (unless image-dired-cmd-rotate-thumbnail-program
- (error "image-dired-cmd-rotate-thumbnail-program is nil"))
+ (image-dired--check-executable-exists
+ 'image-dired-cmd-rotate-thumbnail-program)
(if (not (image-dired-image-at-point-p))
(message "No thumbnail at point")
(let ((file (image-dired-thumb-name
(image-dired-original-file-name)))
@@ -1924,8 +1922,8 @@ image-dired-rotate-original
"Rotate original image DEGREES degrees."
(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"))
+ (image-dired--check-executable-exists
+ 'image-dired-cmd-rotate-original-program)
(let ((file (image-dired-original-file-name))
command)
(unless (eq 'jpeg (image-type file))
@@ -2002,8 +2000,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"))
+ (image-dired--check-executable-exists
+ 'image-dired-cmd-write-exif-data-program)
(let (command)
(setq command (format-spec
image-dired-cmd-write-exif-data-options
@@ -2016,8 +2014,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"))
+ (image-dired--check-executable-exists
+ 'image-dired-cmd-read-exif-data-program)
(let ((buf (get-buffer-create "*image-dired-get-exif-data*"))
command tag-value)
(setq command (format-spec
--
2.9.3
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
Repository revision: 62e4dc4660cb3b29cfffcad0639e51c7f382ced8
^ permalink raw reply related [flat|nested] 10+ messages in thread