emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] problem with size of inline images
@ 2014-08-03 21:54 Joe Corneli
  2014-08-06 13:22 ` Nicolas Goaziou
  0 siblings, 1 reply; 4+ messages in thread
From: Joe Corneli @ 2014-08-03 21:54 UTC (permalink / raw)
  To: emacs-orgmode

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

Hello:

I've noticed a problem around line 19171 of org.el.  The size of
images is supposed to be controlled by attributes and by
`org-image-actual-width'.  But it seems like the use of `when',
`save-match-data' and `string-to-number' are in the wrong order in
this region, so that `(match-string 1)' is not defined properly when
it's evaluated.  The following instructions should allow you to
reproduce the issue:

Run this:
#+BEGIN_SRC shell
wget http://www.lisperati.com/lisplogo_warning_256.png -O lisp_warning.png
#+END_SRC

Then:
[[elisp:(setq org-image-actual-width '(1200))]]
[[elisp:(org-toggle-inline-images)]]

#+ATTR_ORG: :width 256
[[file:./lisp_warning.png]]

... and the attached patch fixed the problem for me.

[-- Attachment #2: save-match-data.patch --]
[-- Type: application/octet-stream, Size: 864 bytes --]

diff --git a/lisp/org.el b/lisp/org.el
index 0f7a4ef..fdfa478 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -19168,13 +19168,14 @@ boundaries."
 			     (when paragraph
 			       (save-excursion
 				 (goto-char (org-element-property :begin paragraph))
-				 (when (save-match-data
-					 (re-search-forward
-					  "^[ \t]*#\\+attr_.*?: +.*?:width +\\(\\S-+\\)"
-					  (org-element-property
-					   :post-affiliated paragraph)
-					  t))
-				   (string-to-number (match-string 1))))))
+				 (save-match-data
+				   (when 
+				       (re-search-forward
+					"^[ \t]*#\\+attr_.*?: +.*?:width +\\(\\S-+\\)"
+					(org-element-property
+					 :post-affiliated paragraph)
+					t)
+				     (string-to-number (match-string 1)))))))
 			   ;; Otherwise, fall-back to provided number.
 			   (car org-image-actual-width)))
 			 ((numberp org-image-actual-width)

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

* Re: [PATCH] problem with size of inline images
  2014-08-03 21:54 [PATCH] problem with size of inline images Joe Corneli
@ 2014-08-06 13:22 ` Nicolas Goaziou
  2014-08-07 12:49   ` Joe Corneli
  0 siblings, 1 reply; 4+ messages in thread
From: Nicolas Goaziou @ 2014-08-06 13:22 UTC (permalink / raw)
  To: Joe Corneli; +Cc: emacs-orgmode

Hello,

Joe Corneli <holtzermann17@gmail.com> writes:

> I've noticed a problem around line 19171 of org.el.  The size of
> images is supposed to be controlled by attributes and by
> `org-image-actual-width'.  But it seems like the use of `when',
> `save-match-data' and `string-to-number' are in the wrong order in
> this region, so that `(match-string 1)' is not defined properly when
> it's evaluated.  The following instructions should allow you to
> reproduce the issue:
>
> Run this:
> #+BEGIN_SRC shell
> wget http://www.lisperati.com/lisplogo_warning_256.png -O lisp_warning.png
> #+END_SRC
>
> Then:
> [[elisp:(setq org-image-actual-width '(1200))]]
> [[elisp:(org-toggle-inline-images)]]
>
> #+ATTR_ORG: :width 256
> [[file:./lisp_warning.png]]
>
> ... and the attached patch fixed the problem for me.

Thanks for your patch.  Would you mind providing a commit message and
send it again with "git format-patch"?  Don't forget to add "TINYCHANGE"
at its end if you haven't signed FSF papers.

> +				 (save-match-data
> +				   (when 
> +				       (re-search-forward
> +					"^[ \t]*#\\+attr_.*?: +.*?:width +\\(\\S-+\\)"
> +					(org-element-property
> +					 :post-affiliated paragraph)
> +					t)
> +				     (string-to-number (match-string 1)))))))

It seems that `save-match-data' is useless anyway and can be removed
altogether. WDYT?


Regards,

-- 
Nicolas Goaziou

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

* Re: [PATCH] problem with size of inline images
  2014-08-06 13:22 ` Nicolas Goaziou
@ 2014-08-07 12:49   ` Joe Corneli
  2014-08-07 13:05     ` Nicolas Goaziou
  0 siblings, 1 reply; 4+ messages in thread
From: Joe Corneli @ 2014-08-07 12:49 UTC (permalink / raw)
  To: Joe Corneli, emacs-orgmode

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

On Wed, Aug 6, 2014 at 2:22 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:

> Thanks for your patch.  Would you mind providing a commit message and
> send it again with "git format-patch"?  Don't forget to add "TINYCHANGE"
> at its end if you haven't signed FSF papers.

Sure.  I did sign them quite some time ago (2003 or 2004) though I
haven't contributed much if any code since then.  Git seems to make
that  quite easy!

> It seems that `save-match-data' is useless anyway and can be removed
> altogether. WDYT?

You're right.  I removed it at another point in the function as well.

[-- Attachment #2: 0001-Fix-bug-associated-with-setting-image-size-via-ATTR.patch --]
[-- Type: application/octet-stream, Size: 1726 bytes --]

From c0f60b932c11936e2c1ed7e84c9385500d3e6e7e Mon Sep 17 00:00:00 2001
From: Joe Corneli <holtzermann17@gmail.com>
Date: Thu, 7 Aug 2014 13:42:54 +0100
Subject: [PATCH] Fix bug associated with setting image size via ATTR.

This change is necessary to make ATTR work.
Removed unnecessary save-match-data forms.
---
 lisp/org.el | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 0f7a4ef..2584dc4 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -19168,13 +19168,13 @@ boundaries."
 			     (when paragraph
 			       (save-excursion
 				 (goto-char (org-element-property :begin paragraph))
-				 (when (save-match-data
-					 (re-search-forward
-					  "^[ \t]*#\\+attr_.*?: +.*?:width +\\(\\S-+\\)"
-					  (org-element-property
-					   :post-affiliated paragraph)
-					  t))
-				   (string-to-number (match-string 1))))))
+				   (when
+				       (re-search-forward
+					"^[ \t]*#\\+attr_.*?: +.*?:width +\\(\\S-+\\)"
+					(org-element-property
+					 :post-affiliated paragraph)
+					t)
+				     (string-to-number (match-string 1))))))
 			   ;; Otherwise, fall-back to provided number.
 			   (car org-image-actual-width)))
 			 ((numberp org-image-actual-width)
@@ -19184,11 +19184,10 @@ boundaries."
 			     'org-image-overlay)))
 		   (if (and (car-safe old) refresh)
 		       (image-refresh (overlay-get (cdr old) 'display))
-		     (let ((image (save-match-data
-				    (create-image file
+		     (let ((image (create-image file
 						  (and width 'imagemagick)
 						  nil
-						  :width width))))
+						  :width width)))
 		       (when image
 			 (let* ((link
 				 ;; If inline image is the description
-- 
1.8.5.2 (Apple Git-48)


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

* Re: [PATCH] problem with size of inline images
  2014-08-07 12:49   ` Joe Corneli
@ 2014-08-07 13:05     ` Nicolas Goaziou
  0 siblings, 0 replies; 4+ messages in thread
From: Nicolas Goaziou @ 2014-08-07 13:05 UTC (permalink / raw)
  To: Joe Corneli; +Cc: emacs-orgmode

Hello,

Joe Corneli <holtzermann17@gmail.com> writes:

> You're right.  I removed it at another point in the function as well.

Applied, with a slight change to commit message. Thank you.


Regards,

-- 
Nicolas Goaziou

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

end of thread, other threads:[~2014-08-07 13:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-03 21:54 [PATCH] problem with size of inline images Joe Corneli
2014-08-06 13:22 ` Nicolas Goaziou
2014-08-07 12:49   ` Joe Corneli
2014-08-07 13:05     ` 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).