Re: [PATCH]

[Timothy <tecosaur@gmail.com>]

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

Hi  Bastien,

Thanks for taking a look at my patch.

Bastien <bzg@gnu.org> writes:

> On a first look, it seems a bit hackish, but this is probably useful.

Taking a look at the commit, I can see how it doesn’t look particularly tidy. I
think this is more a function of the way that function is structured than my
change though. As such, I’ve had a second look at the code, and given how large
that function is, split off the width calculation into a new function, where
I’ve re-implemented the logic in (IMO) a cleaner way.

So now, in the original function there’s just
┌────
│ (let ((width (org-display-inline-image--width link))
└────

See the attached patch for the new function `org-display-inline-image--width' (to
be applied on top of my previous patch). Oh, and I’ve also added support for
`:width 70%'.

All the best,
Timothy

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0003-org-Support-displaying-X-width-images.patch --]
[-- Type: text/x-patch, Size: 1916 bytes --]

From d9c83a962c0ce26e3d7baf2a5b7a58ba054ef275 Mon Sep 17 00:00:00 2001
From: TEC <tec@tecosaur.com>
Date: Mon, 27 Sep 2021 19:16:58 +0800
Subject: [PATCH 3/3] org: Support displaying X% width images

* lisp/org.el (org-display-inline-image--width): Instead of interpreting
an image :width of X% as X pixels, take it as X% of the text width of
the buffer.
---
 lisp/org.el | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index d61e74572..829df8cae 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -16647,7 +16647,8 @@ (defun org-display-inline-image--width (link)
 - When `org-image-actual-width' is t, the image's pixel width is used.
 - When `org-image-actual-width' is a number, that value will is used.
 - When `org-image-actual-width' is nil or a list, the first :width attribute
-  set (if it exists) is used to set the image width.
+  set (if it exists) is used to set the image width.  A width of X% is
+  divided by 100.
   If no :width attribute is given and `org-image-actual-width' is a list with
   a number as the car, then that number is used as the default value.
   If the value is a float between 0 and 2, it interpreted as that proportion
@@ -16667,7 +16668,11 @@ (defun org-display-inline-image--width (link)
                              (re-search-forward attr-re par-end t)))
               (string-to-number (match-string 1))))
            (attr-width-val
-            (when attr-width (string-to-number attr-width)))
+            (cond
+             ((null attr-width) nil)
+             ((string-match-p "\\`[0-9.]+%")
+              (/ (string-to-number attr-width) 100.0))
+             (t (string-to-number attr-width))))
            ;; Fallback to `org-image-actual-width' if no explicit width is given.
            (width (or attr-width-val (car org-image-actual-width))))
       (if (and (floatp width) (<= 0 width 2.0))
-- 
2.33.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-org-Refactor-width-in-org-display-inline-images.patch --]
[-- Type: text/x-patch, Size: 6646 bytes --]

From 1fd5e43137a34418c149240b15fd4bdc311f4fd3 Mon Sep 17 00:00:00 2001
From: TEC <tec@tecosaur.com>
Date: Mon, 27 Sep 2021 18:46:03 +0800
Subject: [PATCH 2/3] org: Refactor width in `org-display-inline-images'

* lisp/org.el (org-display-inline-images,
org-display-inline-image--width): Extract the width determination in
`org-display-inline-images' into a new function
`org-display-inline-image--width' where I have taken the opportunity to
refactor the width-determination code.
---
 lisp/org.el | 85 +++++++++++++++++++++++++++++------------------------
 1 file changed, 47 insertions(+), 38 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 1a1feda78..d61e74572 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -16617,44 +16617,7 @@ (defun org-display-inline-images (&optional include-linked refresh beg end)
 				  (ignore-errors (org-attach-expand path)))
                               (expand-file-name path))))
 		  (when (and file (file-exists-p file))
-		    (let ((width
-			   ;; Apply `org-image-actual-width' specifications.
-			   (cond
-			    ((eq org-image-actual-width t) nil)
-			    ((listp org-image-actual-width)
-                             (let ((width
-                                    (or
-                                     ;; First try to find a width among
-                                     ;; attributes associated to the paragraph
-                                     ;; containing link.
-                                     (pcase (org-element-lineage link '(paragraph))
-                                       (`nil nil)
-                                       (par (let* ((case-fold-search t)
-                                                   (end (org-element-property :post-affiliated par))
-                                                   (re "^[ \t]*#\\+attr_.*?: +.*?:width +\\(\\S-+\\)"))
-                                              (when (org-with-point-at
-                                                        (org-element-property :begin par)
-                                                      (re-search-forward re end t))
-                                                (string-to-number (match-string 1)))))))
-                                    ;; Otherwise, fall-back to provided number.
-                                    (car org-image-actual-width))))
-                             (if (and (floatp width) (<= 0 width 2.0))
-                                 ;; A float in [0,2] should be interpereted as this portion of
-                                 ;; the text width in the window.  This works well with cases like
-                                 ;; #+attr_latex: :width 0.X\{line,page,column,etc.}width,
-                                 ;; as the "0.X" is pulled out as a float.  We use 2 as the upper
-                                 ;; bound as cases such as 1.2\linewidth are feasible.
-                                 (round (* width
-                                           (window-pixel-width)
-                                           (/ (or (and (bound-and-true-p visual-fill-column-mode)
-                                                       (or visual-fill-column-width auto-fill-function))
-                                                  (when auto-fill-function fill-column)
-                                                  (window-text-width))
-                                              (float (window-total-width)))))
-                               width))
-			    ((numberp org-image-actual-width)
-			     org-image-actual-width)
-			    (t nil)))
+		    (let ((width (org-display-inline-image--width link))
 			  (old (get-char-property-and-overlay
 				(org-element-property :begin link)
 				'org-image-overlay)))
@@ -16679,6 +16642,52 @@ (defun org-display-inline-images (&optional include-linked refresh beg end)
 				(overlay-put ov 'keymap image-map))
 			      (push ov org-inline-image-overlays))))))))))))))))
 
+(defun org-display-inline-image--width (link)
+  "Determine the display width of the image LINK, in pixels.
+- When `org-image-actual-width' is t, the image's pixel width is used.
+- When `org-image-actual-width' is a number, that value will is used.
+- When `org-image-actual-width' is nil or a list, the first :width attribute
+  set (if it exists) is used to set the image width.
+  If no :width attribute is given and `org-image-actual-width' is a list with
+  a number as the car, then that number is used as the default value.
+  If the value is a float between 0 and 2, it interpreted as that proportion
+  of the text width in the buffer."
+  ;; Apply `org-image-actual-width' specifications.
+  (cond
+   ((eq org-image-actual-width t) nil)
+   ((listp org-image-actual-width)
+    (let* ((case-fold-search t)
+           (par (org-element-lineage link '(paragraph)))
+           (attr-re "^[ \t]*#\\+attr_.*?: +.*?:width +\\(\\S-+\\)")
+           (par-end (org-element-property :post-affiliated par))
+           ;; Try to find an attribute providing a :width.
+           (attr-width
+            (when (and par (org-with-point-at
+                               (org-element-property :begin par)
+                             (re-search-forward attr-re par-end t)))
+              (string-to-number (match-string 1))))
+           (attr-width-val
+            (when attr-width (string-to-number attr-width)))
+           ;; Fallback to `org-image-actual-width' if no explicit width is given.
+           (width (or attr-width-val (car org-image-actual-width))))
+      (if (and (floatp width) (<= 0 width 2.0))
+          ;; A float in [0,2] should be interpereted as this portion of
+          ;; the text width in the window.  This works well with cases like
+          ;; #+attr_latex: :width 0.X\{line,page,column,etc.}width,
+          ;; as the "0.X" is pulled out as a float.  We use 2 as the upper
+          ;; bound as cases such as 1.2\linewidth are feasible.
+          (round (* width
+                    (window-pixel-width)
+                    (/ (or (and (bound-and-true-p visual-fill-column-mode)
+                                (or visual-fill-column-width auto-fill-function))
+                           (when auto-fill-function fill-column)
+                           (window-text-width))
+                       (float (window-total-width)))))
+        width)))
+   ((numberp org-image-actual-width)
+    org-image-actual-width)
+   (t nil)))
+
 (defun org-display-inline-remove-overlay (ov after _beg _end &optional _len)
   "Remove inline-display overlay if a corresponding region is modified."
   (let ((inhibit-modification-hooks t))
-- 
2.33.0