* Bug: org-string-display throws on image display property [9.1.9 (release_9.1.9-65-g5e4542 @ /home/blc/.local/share/emacs/27.0.50/lisp/org/)]
@ 2018-09-02 0:28 Basil L. Contovounesios
2018-09-03 15:27 ` Nicolas Goaziou
0 siblings, 1 reply; 4+ messages in thread
From: Basil L. Contovounesios @ 2018-09-02 0:28 UTC (permalink / raw)
To: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 285 bytes --]
Severity: minor
0. emacs -Q
1. M-x toggle-debug-on-error RET
2. Evaluate the following:
(require 'org)
(org-string-display
(propertize " " 'display '(image :type svg
:file "/path/to/image.svg")))
This gives the following error backtrace:
[-- Attachment #2: Error backtrace --]
[-- Type: text/plain, Size: 1217 bytes --]
Debugger entered--Lisp error: (wrong-type-argument sequencep t)
#f(compiled-function (s property filter) #<bytecode 0x394179>)(t invisible #f(compiled-function (props) #<bytecode 0x394185>))
#f(compiled-function (s) #<bytecode 0x11718f5>)(t)
#f(compiled-function (props) #<bytecode 0x1171905>)((display (image :type svg :file "/path/to/image.svg")))
#f(compiled-function (s property filter) #<bytecode 0x394179>)(#(" " 0 1 (display (image :type svg :file "/path/to/image.svg"))) display #f(compiled-function (props) #<bytecode 0x1171905>))
#f(compiled-function (build-from-parts prune-invisible s) #<bytecode 0x3940dd>)(#f(compiled-function (s property filter) #<bytecode 0x394179>) #f(compiled-function (s) #<bytecode 0x11718f5>) #(" " 0 1 (display (image :type svg :file "/path/to/image.svg"))))
org-string-display(#(" " 0 1 (display (image :type svg :file "/path/to/image.svg"))))
eval((org-string-display (propertize " " 'display '(image :type svg :file "/path/to/image.svg"))) nil)
elisp--eval-last-sexp(t)
eval-last-sexp(t)
eval-print-last-sexp(nil)
funcall-interactively(eval-print-last-sexp nil)
call-interactively(eval-print-last-sexp nil nil)
command-execute(eval-print-last-sexp)
[-- Attachment #3: Type: text/plain, Size: 292 bytes --]
I realise that org-string-display is a relatively obscure function that
was probably not designed with image or other complex display properties
in mind, but AFAICT the following is a pretty obvious "fix", given that
the surrounding code expects 'value' to be a string or nil, but never t:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: org-string-display.diff --]
[-- Type: text/x-diff, Size: 520 bytes --]
diff --git a/lisp/org/org-macs.el b/lisp/org/org-macs.el
index 583633605f..5ab069e6a2 100644
--- a/lisp/org/org-macs.el
+++ b/lisp/org/org-macs.el
@@ -108,7 +108,7 @@ org-string-display
;; string (face...).
(let* ((display (plist-get props 'display))
(value (if (stringp display) display
- (cl-some #'stringp display))))
+ (cl-find-if #'stringp display))))
(when value
(apply #'propertize
;; Displayed string could contain
[-- Attachment #5: Type: text/plain, Size: 416 bytes --]
With this change, the return value of org-string-display does not really
make sense for images, but at least the function doesn't barf on complex
but valid display properties. WDYT?
Thanks,
--
Basil
Emacs : GNU Emacs 27.0.50 (build 3, x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
of 2018-09-01
Package: Org mode version 9.1.9 (release_9.1.9-65-g5e4542 @ /home/blc/.local/share/emacs/27.0.50/lisp/org/)
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: Bug: org-string-display throws on image display property [9.1.9 (release_9.1.9-65-g5e4542 @ /home/blc/.local/share/emacs/27.0.50/lisp/org/)]
2018-09-02 0:28 Bug: org-string-display throws on image display property [9.1.9 (release_9.1.9-65-g5e4542 @ /home/blc/.local/share/emacs/27.0.50/lisp/org/)] Basil L. Contovounesios
@ 2018-09-03 15:27 ` Nicolas Goaziou
2018-09-03 16:46 ` Basil L. Contovounesios
0 siblings, 1 reply; 4+ messages in thread
From: Nicolas Goaziou @ 2018-09-03 15:27 UTC (permalink / raw)
To: Basil L. Contovounesios; +Cc: emacs-orgmode
Hello,
"Basil L. Contovounesios" <contovob@tcd.ie> writes:
> Severity: minor
>
> 0. emacs -Q
> 1. M-x toggle-debug-on-error RET
> 2. Evaluate the following:
>
> (require 'org)
> (org-string-display
> (propertize " " 'display '(image :type svg
> :file "/path/to/image.svg")))
>
> This gives the following error backtrace:
>
> Debugger entered--Lisp error: (wrong-type-argument sequencep t)
`org-string-display' doesn't exist anymore in master (Org 9.2). However,
there is the same error with `org-string-width'.
What is `org-string-width' supposed to do on your example? Probably
return an error.
> I realise that org-string-display is a relatively obscure function that
> was probably not designed with image or other complex display properties
> in mind, but AFAICT the following is a pretty obvious "fix", given that
> the surrounding code expects 'value' to be a string or nil, but never t:
>
> diff --git a/lisp/org/org-macs.el b/lisp/org/org-macs.el
> index 583633605f..5ab069e6a2 100644
> --- a/lisp/org/org-macs.el
> +++ b/lisp/org/org-macs.el
> @@ -108,7 +108,7 @@ org-string-display
> ;; string (face...).
> (let* ((display (plist-get props 'display))
> (value (if (stringp display) display
> - (cl-some #'stringp display))))
> + (cl-find-if #'stringp display))))
> (when value
> (apply #'propertize
> ;; Displayed string could contain
>
>
> With this change, the return value of org-string-display does not really
> make sense for images, but at least the function doesn't barf on complex
> but valid display properties. WDYT?
If `org-string-width' returns nil, it will ensue an error somewhere
further in the code. So returning an error right here is better (or
find a way to compute the width of an image in columns).
As another data point, `string-width' returns 1 on your example, which
is probably wrong.
WDYT?
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Bug: org-string-display throws on image display property [9.1.9 (release_9.1.9-65-g5e4542 @ /home/blc/.local/share/emacs/27.0.50/lisp/org/)]
2018-09-03 15:27 ` Nicolas Goaziou
@ 2018-09-03 16:46 ` Basil L. Contovounesios
2018-10-25 13:43 ` Nicolas Goaziou
0 siblings, 1 reply; 4+ messages in thread
From: Basil L. Contovounesios @ 2018-09-03 16:46 UTC (permalink / raw)
To: Nicolas Goaziou; +Cc: emacs-orgmode
Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:
> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
>> Severity: minor
>>
>> 0. emacs -Q
>> 1. M-x toggle-debug-on-error RET
>> 2. Evaluate the following:
>>
>> (require 'org)
>> (org-string-display
>> (propertize " " 'display '(image :type svg
>> :file "/path/to/image.svg")))
>>
>> This gives the following error backtrace:
>>
>> Debugger entered--Lisp error: (wrong-type-argument sequencep t)
>
> `org-string-display' doesn't exist anymore in master (Org 9.2). However,
> there is the same error with `org-string-width'.
>
> What is `org-string-width' supposed to do on your example? Probably
> return an error.
I realise that the 'display' property is a pretty complicated beast, so
it may be impractical for 'org-string-width' to do this, but I think
more accurate/useful behaviour would be to calculate the displayed width
of the image via 'image-size' or 'image-display-size', rather than
signalling an error.
>> I realise that org-string-display is a relatively obscure function that
>> was probably not designed with image or other complex display properties
>> in mind, but AFAICT the following is a pretty obvious "fix", given that
>> the surrounding code expects 'value' to be a string or nil, but never t:
>>
>> diff --git a/lisp/org/org-macs.el b/lisp/org/org-macs.el
>> index 583633605f..5ab069e6a2 100644
>> --- a/lisp/org/org-macs.el
>> +++ b/lisp/org/org-macs.el
>> @@ -108,7 +108,7 @@ org-string-display
>> ;; string (face...).
>> (let* ((display (plist-get props 'display))
>> (value (if (stringp display) display
>> - (cl-some #'stringp display))))
>> + (cl-find-if #'stringp display))))
>> (when value
>> (apply #'propertize
>> ;; Displayed string could contain
>>
>>
>> With this change, the return value of org-string-display does not really
>> make sense for images, but at least the function doesn't barf on complex
>> but valid display properties. WDYT?
>
> If `org-string-width' returns nil, it will ensue an error somewhere
> further in the code. So returning an error right here is better (or
> find a way to compute the width of an image in columns).
Why/when would 'org-string-width' return nil? My suggestion was to
replace 'cl-some' with 'cl-find-if', because the value of
(cl-some #'stringp ...) is a boolean, not a sequence.
To clarify - I am not suggesting that 'org-string-width' should never
signal an error; I am only suggesting that the current
(wrong-type-argument sequencep t) error is due to inappropriate use of
'cl-some'. My comments may already be outdated, though, since they
refer to the version of org-macs.el shipping with latest Emacs master.
> As another data point, `string-width' returns 1 on your example, which
> is probably wrong.
Neither 'string-width' nor 'current-column' take 'display' properties
into account; see e.g. https://emacs.stackexchange.com/q/44495/15748.
But that's an Emacs feature request of its own.
Thanks,
--
Basil
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Bug: org-string-display throws on image display property [9.1.9 (release_9.1.9-65-g5e4542 @ /home/blc/.local/share/emacs/27.0.50/lisp/org/)]
2018-09-03 16:46 ` Basil L. Contovounesios
@ 2018-10-25 13:43 ` Nicolas Goaziou
0 siblings, 0 replies; 4+ messages in thread
From: Nicolas Goaziou @ 2018-10-25 13:43 UTC (permalink / raw)
To: Basil L. Contovounesios; +Cc: emacs-orgmode
Hello,
"Basil L. Contovounesios" <contovob@tcd.ie> writes:
> I realise that the 'display' property is a pretty complicated beast, so
> it may be impractical for 'org-string-width' to do this, but I think
> more accurate/useful behaviour would be to calculate the displayed width
> of the image via 'image-size' or 'image-display-size', rather than
> signalling an error.
Fixed. `org-string-width' has now a limited support for images.
Thank you.
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-10-25 13:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-02 0:28 Bug: org-string-display throws on image display property [9.1.9 (release_9.1.9-65-g5e4542 @ /home/blc/.local/share/emacs/27.0.50/lisp/org/)] Basil L. Contovounesios
2018-09-03 15:27 ` Nicolas Goaziou
2018-09-03 16:46 ` Basil L. Contovounesios
2018-10-25 13:43 ` Nicolas Goaziou
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.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).