unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: master ca47390: image-dired: Report when a necessary executable is not found
       [not found] ` <20160904134431.7513F220140@vcs.savannah.gnu.org>
@ 2016-09-04 16:41   ` Glenn Morris
  2016-09-05  3:42     ` Tino Calancha
  0 siblings, 1 reply; 10+ messages in thread
From: Glenn Morris @ 2016-09-04 16:41 UTC (permalink / raw)
  To: emacs-devel; +Cc: Tino Calancha


Hi,


Tino Calancha wrote:

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

Please update this and all the other affected :types to allow for the
nil values that may now occur. You might also want to document what a
value of nil means. Thanks.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: master ca47390: image-dired: Report when a necessary executable is not found
  2016-09-04 16:41   ` master ca47390: image-dired: Report when a necessary executable is not found Glenn Morris
@ 2016-09-05  3:42     ` Tino Calancha
  2016-09-05  4:42       ` Clément Pit--Claudel
  2016-09-12 18:44       ` Glenn Morris
  0 siblings, 2 replies; 10+ messages in thread
From: Tino Calancha @ 2016-09-05  3:42 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Tino Calancha, Emacs developers


On Sun, 4 Sep 2016, Glenn Morris wrote:

>
> Hi,
>
>
> Tino Calancha wrote:
>
>>  (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
>
> Please update this and all the other affected :types to allow for the
> nil values that may now occur. You might also want to document what a
> value of nil means. Thanks.
Please let me know if the following patch is OK:

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From e8d540387d92c64b971a31f372fd4627c5198948 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Mon, 5 Sep 2016 12:32:42 +0900
Subject: [PATCH] image-dired: Update :type declaration for several options

* lisp/image-dired.el (image-dired-cmd-create-thumbnail-program)
(image-dired-cmd-create-temp-image-program)
(image-dired-cmd-rotate-thumbnail-program)
(image-dired-cmd-rotate-original-program)
(image-dired-cmd-write-exif-data-program)
(image-dired-cmd-read-exif-data-program):
Update :type to allow for a nil value.
Mention in the doc string that if the value is nil, then
the function using this option signals an error.
---
  lisp/image-dired.el | 32 +++++++++++++++++++-------------
  1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/lisp/image-dired.el b/lisp/image-dired.el
index 5ac4600..eddaee1 100644
--- a/lisp/image-dired.el
+++ b/lisp/image-dired.el
@@ -224,10 +224,11 @@ image-dired-gallery-thumb-image-root-url
    :group 'image-dired)

  (defcustom image-dired-cmd-create-thumbnail-program
-  (executable-find "convert")
+  (when (executable-find "convert") "convert")
    "Executable used to create thumbnail.
+If nil, `image-dired-create-thumb' signals an error.
  Used together with `image-dired-cmd-create-thumbnail-options'."
-  :type 'string
+  :type '(choice (const :tag "Not Set" nil) string)
    :group 'image-dired)

  (defcustom image-dired-cmd-create-thumbnail-options
@@ -242,10 +243,11 @@ image-dired-cmd-create-thumbnail-options
    :group 'image-dired)

  (defcustom image-dired-cmd-create-temp-image-program
-  (executable-find "convert")
+  (when (executable-find "convert") "convert")
    "Executable used to create temporary image.
+If nil, `image-dired-display-image' signals an error.
  Used together with `image-dired-cmd-create-temp-image-options'."
-  :type 'string
+  :type '(choice (const :tag "Not Set" nil) string)
    :group 'image-dired)

  (defcustom image-dired-cmd-create-temp-image-options
@@ -308,10 +310,11 @@ image-dired-cmd-create-standard-thumbnail-command
    :group 'image-dired)

  (defcustom image-dired-cmd-rotate-thumbnail-program
-  (executable-find "mogrify")
+  (when (executable-find "mogrify") "mogrify")
    "Executable used to rotate thumbnail.
+If nil, `image-dired-rotate-thumbnail' signals an error.
  Used together with `image-dired-cmd-rotate-thumbnail-options'."
-  :type 'string
+  :type '(choice (const :tag "Not Set" nil) string)
    :group 'image-dired)

  (defcustom image-dired-cmd-rotate-thumbnail-options
@@ -326,11 +329,12 @@ image-dired-cmd-rotate-thumbnail-options
    :group 'image-dired)

  (defcustom image-dired-cmd-rotate-original-program
-  (cond ((executable-find "jpegtran"))
-        ((executable-find "convert")))
+  (cond ((executable-find "jpegtran") "jpegtran")
+        ((executable-find "convert") "convert"))
    "Executable used to rotate original image.
+If nil, `image-dired-rotate-original' signals an error.
  Used together with `image-dired-cmd-rotate-original-options'."
-  :type 'string
+  :type '(choice (const :tag "Not Set" nil) string)
    :group 'image-dired)

  (defcustom image-dired-cmd-rotate-original-options
@@ -364,10 +368,11 @@ image-dired-rotate-original-ask-before-overwrite
    :group 'image-dired)

  (defcustom image-dired-cmd-write-exif-data-program
-  (executable-find "exiftool")
+  (when (executable-find "exiftool") "exiftool")
    "Program used to write EXIF data to image.
+If nil, `image-dired-set-exif-data' signals an error.
  Used together with `image-dired-cmd-write-exif-data-options'."
-  :type 'string
+  :type '(choice (const :tag "Not Set" nil) string)
    :group 'image-dired)

  (defcustom image-dired-cmd-write-exif-data-options
@@ -381,10 +386,11 @@ image-dired-cmd-write-exif-data-options
    :group 'image-dired)

  (defcustom image-dired-cmd-read-exif-data-program
-  (executable-find "exiftool")
+  (when (executable-find "exiftool") "exiftool")
    "Program used to read EXIF data to image.
+If nil, `image-dired-get-exif-data' signals an error.
  Used together with `image-dired-cmd-read-exif-data-program-options'."
-  :type 'string
+  :type '(choice (const :tag "Not Set" nil) string)
    :group 'image-dired)

  (defcustom image-dired-cmd-read-exif-data-options
-- 
2.9.3


;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;




^ 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  3:42     ` Tino Calancha
@ 2016-09-05  4:42       ` Clément Pit--Claudel
  2016-09-05  5:30         ` Tino Calancha
  2016-09-05 18:00         ` Tino Calancha
  2016-09-12 18:44       ` Glenn Morris
  1 sibling, 2 replies; 10+ messages in thread
From: Clément Pit--Claudel @ 2016-09-05  4:42 UTC (permalink / raw)
  To: emacs-devel


[-- Attachment #1.1: Type: text/plain, Size: 784 bytes --]

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

Cheers,
Clément.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[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  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  5:30         ` Tino Calancha
@ 2016-09-05  6:41           ` Andreas Schwab
  2016-09-05  7:12             ` Tino Calancha
  2016-09-05 15:48           ` Clément Pit--Claudel
  1 sibling, 1 reply; 10+ messages in thread
From: Andreas Schwab @ 2016-09-05  6:41 UTC (permalink / raw)
  To: Tino Calancha; +Cc: Clément Pit--Claudel, Emacs developers

On Sep 05 2016, Tino Calancha <tino.calancha@gmail.com> wrote:

> @@ -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))

What does this hunk have to do with the subject?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: master ca47390: image-dired: Report when a necessary executable is not found
  2016-09-05  6:41           ` Andreas Schwab
@ 2016-09-05  7:12             ` Tino Calancha
  0 siblings, 0 replies; 10+ messages in thread
From: Tino Calancha @ 2016-09-05  7:12 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Emacs developers, Clément Pit--Claudel, Tino Calancha

[-- Attachment #1: Type: text/plain, Size: 564 bytes --]



On Mon, 5 Sep 2016, Andreas Schwab wrote:

> On Sep 05 2016, Tino Calancha <tino.calancha@gmail.com> wrote:
>
>> @@ -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))
>
> What does this hunk have to do with the subject?
Nothing.  That is to avoid the compilation warning:
‘next-line’ is for interactive use only; use ‘forward-line’ instead.

I'm sorry for the confusion.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: master ca47390: image-dired: Report when a necessary executable is not found
  2016-09-05  5:30         ` Tino Calancha
  2016-09-05  6:41           ` Andreas Schwab
@ 2016-09-05 15:48           ` Clément Pit--Claudel
  1 sibling, 0 replies; 10+ messages in thread
From: Clément Pit--Claudel @ 2016-09-05 15:48 UTC (permalink / raw)
  To: Tino Calancha; +Cc: Emacs developers


[-- Attachment #1.1: Type: text/plain, Size: 506 bytes --]

On 2016-09-05 01:30, Tino Calancha wrote:
> 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:

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.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[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

* Re: master ca47390: image-dired: Report when a necessary executable is not found
  2016-09-05  3:42     ` Tino Calancha
  2016-09-05  4:42       ` Clément Pit--Claudel
@ 2016-09-12 18:44       ` Glenn Morris
  2016-09-12 19:44         ` Tino Calancha
  1 sibling, 1 reply; 10+ messages in thread
From: Glenn Morris @ 2016-09-12 18:44 UTC (permalink / raw)
  To: Tino Calancha; +Cc: Emacs developers

Tino Calancha wrote:

>    "Executable used to create thumbnail.
> +If nil, `image-dired-create-thumb' signals an error.

I think you could just say:

  "Executable used to create thumbnail, or nil if not found."

Otherwise, LGTM, thanks.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: master ca47390: image-dired: Report when a necessary executable is not found
  2016-09-12 18:44       ` Glenn Morris
@ 2016-09-12 19:44         ` Tino Calancha
  0 siblings, 0 replies; 10+ messages in thread
From: Tino Calancha @ 2016-09-12 19:44 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Emacs developers, Tino Calancha

[-- Attachment #1: Type: text/plain, Size: 741 bytes --]



On Mon, 12 Sep 2016, Glenn Morris wrote:

> Tino Calancha wrote:
>
>>    "Executable used to create thumbnail.
>> +If nil, `image-dired-create-thumb' signals an error.
>
> I think you could just say:
>
>  "Executable used to create thumbnail, or nil if not found."
That part was reverted after a suggestion from Clément in:
https://lists.gnu.org/archive/html/emacs-devel/2016-09/msg00144.html
I kept those definitions as the original, i.e., just strings, no call
to `executable-find' there.  The check for existing executables is
performed in a new function image-dired--check-executable-exists
which is called on each function using an external program.

> Otherwise, LGTM, thanks.
Thank you very much.
Pushed to master as commit 5d7433ab

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-09-12 19:44 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20160904134431.30494.94659@vcs.savannah.gnu.org>
     [not found] ` <20160904134431.7513F220140@vcs.savannah.gnu.org>
2016-09-04 16:41   ` master ca47390: image-dired: Report when a necessary executable is not found Glenn Morris
2016-09-05  3:42     ` Tino Calancha
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  7:12             ` Tino Calancha
2016-09-05 15:48           ` Clément Pit--Claudel
2016-09-05 18:00         ` Tino Calancha
2016-09-12 18:44       ` Glenn Morris
2016-09-12 19:44         ` 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).