emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* bug#47088: org-w3m: handle w3m-image link information
       [not found] <20210312062614.usandeuwplmn7fu3@E15-2016.optimum.net>
@ 2021-05-05  8:16 ` Bastien
  2021-05-06 22:05   ` Boruch Baum
  0 siblings, 1 reply; 6+ messages in thread
From: Bastien @ 2021-05-05  8:16 UTC (permalink / raw)
  To: Boruch Baum; +Cc: 47088

Boruch Baum <boruch_baum@gmx.com> writes:

> Attached is a patch that adds to function `org-w3m-copy-for-org-mode'
> support for w3m-image links. Note that because function
> `org-make-link-string' is used for constructing the buffer substring to
> be copied, w3m's special face to distinguish an image link is not
> transfered.

Thanks, it looks good.

Can you try updating the patch against Org's upstream repository at
https://code.orgmode.org/bzg/org-mode/?

Note that the file is lisp/ol-w3m.el there.

-- 
 Bastien




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

* bug#47088: org-w3m: handle w3m-image link information
  2021-05-05  8:16 ` bug#47088: org-w3m: handle w3m-image link information Bastien
@ 2021-05-06 22:05   ` Boruch Baum
  2021-05-06 23:56     ` Nick Savage
  2021-05-09  6:50     ` Bastien
  0 siblings, 2 replies; 6+ messages in thread
From: Boruch Baum @ 2021-05-06 22:05 UTC (permalink / raw)
  To: Bastien; +Cc: 47088

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

On 2021-05-05 10:16, Bastien wrote:
> Thanks, it looks good.
>
> Can you try updating the patch against Org's upstream repository at
> https://code.orgmode.org/bzg/org-mode/?
>
> Note that the file is lisp/ol-w3m.el there.

1] Attached.

2] Here's a commit message in the Changelog style:

   2021-05-06  Boruch Baum  <boruch_baum@gmx.com>

	* ol-w3m.el (org-w3m-copy-for-org-mode)
	(org-w3m-get-next-link-start, org-w3m-get-prev-link-start):
	Account for w3m-img links.
	(org-w3m-get-anchor-start, org-w3m-get-prev-link-start)
	(org-w3m-no-prev-link-p): Unused function notes.
	(org-w3m-get-image-end): New function, for w3m-img links.

3] As mentioned in the patch, and in the commit message, there seem to
   be several functioned unused in the code base that could be
   candidates for removal...

        org-w3m-get-anchor-start
        org-w3m-no-prev-link-p
        org-w3m-get-prev-link-start

4] Additionally, in performing the merge I came across function
   org-string-nw-p, which seems to be an unnecessary duplicate of the
   emacs core function string-blank-p. It may be that historically you
   once needed your own home-made functions/macros, but now that emacs
   core has them, you may want to consider a refactor.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

[-- Attachment #2: ol-w3m.patch --]
[-- Type: text/x-diff, Size: 7208 bytes --]

diff --git a/ol-w3m.el b/ol-w3m.el
index ebb11ce..2738e01 100644
--- a/ol-w3m.el
+++ b/ol-w3m.el
@@ -82,26 +82,41 @@ so that it can be yanked into an Org  buffer with links working correctly."
         (setq temp-position (point))
         ;; move to next anchor when current point is not at anchor
         (or (get-text-property (point) 'w3m-href-anchor) (org-w3m-get-next-link-start))
-        (if (<= (point) transform-end) ; if point is inside transform bound
-            (progn
-              ;; get content between two links.
-              (when (> (point) temp-position)
-                (setq return-content (concat return-content
-                                             (buffer-substring
-                                              temp-position (point)))))
-              ;; get link location at current point.
-              (setq link-location (get-text-property (point) 'w3m-href-anchor))
-              ;; get link title at current point.
-              (setq link-title (buffer-substring (point)
-                                                 (org-w3m-get-anchor-end)))
-              ;; concat Org style url to `return-content'.
-              (setq return-content
-		    (concat return-content
-			    (if (org-string-nw-p link-location)
-				(org-link-make-string link-location link-title)
-			      link-title))))
+        (cond
+         ((<= (point) transform-end) ; point is inside transform bound
+           ;; get content between two links.
+           (when (> (point) temp-position)
+             (setq return-content (concat return-content
+                                          (buffer-substring
+                                           temp-position (point)))))
+           (cond
+            ((setq link-location (get-text-property (point) 'w3m-href-anchor))
+             ;; current point is a link
+             ;; (we thus also got link location at current point)
+             ;; get link title at current point.
+             (setq link-title (buffer-substring (point)
+                                                (org-w3m-get-anchor-end)))
+             ;; concat Org style url to `return-content'.
+             (setq return-content
+               (concat return-content
+                       (if (org-string-nw-p link-location)
+                         (org-link-make-string link-location link-title)
+                        link-title))))
+            ((setq link-location (get-text-property (point) 'w3m-image))
+             ;; current point is an image
+             ;; (we thus also got image link location at current point)
+             ;; get link title at current point.
+             (setq link-title (buffer-substring (point) (org-w3m-get-image-end)))
+             ;; concat Org style url to `return-content'.
+             (setq return-content
+               (concat return-content
+                       (if (org-string-nw-p link-location)
+                         (org-link-make-string link-location link-title)
+                        link-title))))
+            (t nil))); current point is neither a link nor an image
+         (t ; point is NOT inside transform bound
           (goto-char temp-position) ; reset point before jump next anchor
-          (setq out-bound t)))	    ; for break out `while' loop
+          (setq out-bound t))))	    ; for break out `while' loop
       ;; add the rest until end of the region to be copied
       (when (< (point) transform-end)
         (setq return-content
@@ -114,6 +129,7 @@ so that it can be yanked into an Org  buffer with links working correctly."
 (defun org-w3m-get-anchor-start ()
   "Move cursor to the start of current anchor.  Return point."
   ;; get start position of anchor or current point
+  ;; NOTE: This function seems never to be used. Should it be removed?
   (goto-char (or (previous-single-property-change (point) 'w3m-anchor-sequence)
                  (point))))

@@ -123,26 +139,46 @@ so that it can be yanked into an Org  buffer with links working correctly."
   (goto-char (or (next-single-property-change (point) 'w3m-anchor-sequence)
 		 (point))))

+(defun org-w3m-get-image-end ()
+  "Move cursor to the end of current image.  Return point."
+  ;; get end position of image or point
+  ;; NOTE: Function `org-w3m-get-image-start' was not created because
+  ;;       function `org-w3m-get-anchor-start' is never used.
+  (goto-char (or (next-single-property-change (point) 'w3m-image)
+		 (point))))
+
 (defun org-w3m-get-next-link-start ()
-  "Move cursor to the start of next link.  Return point."
-  (catch 'reach
-    (while (next-single-property-change (point) 'w3m-anchor-sequence)
-      ;; jump to next anchor
-      (goto-char (next-single-property-change (point) 'w3m-anchor-sequence))
-      (when (get-text-property (point) 'w3m-href-anchor)
-        ;; return point when current is valid link
-        (throw 'reach nil))))
-  (point))
+  "Move cursor to the start of next link or image.  Return point."
+  (let (pos start-pos anchor-pos image-pos)
+    (setq pos (setq start-pos (point)))
+    (setq anchor-pos
+      (catch 'reach
+        (while (setq pos (next-single-property-change pos 'w3m-anchor-sequence))
+          (when (get-text-property pos 'w3m-href-anchor)
+            (throw 'reach pos)))))
+    (setq pos start-pos)
+    (setq image-pos
+      (catch 'reach
+        (while (setq pos (next-single-property-change pos 'w3m-image))
+          (when (get-text-property pos 'w3m-image)
+            (throw 'reach pos)))))
+    (goto-char (min (or anchor-pos (point-max)) (or image-pos (point-max))))))

 (defun org-w3m-get-prev-link-start ()
   "Move cursor to the start of previous link.  Return point."
+  ;; NOTE: This function is only called by `org-w3m-no-prev-link-p',
+  ;;       which itself seems never to be used. Should it be removed?
+  ;;
+  ;; WARNING: This function has not been updated to account for
+  ;;      `w3m-image'. See `org-w3m-get-next-link-start'.
   (catch 'reach
-    (while (previous-single-property-change (point) 'w3m-anchor-sequence)
-      ;; jump to previous anchor
-      (goto-char (previous-single-property-change (point) 'w3m-anchor-sequence))
-      (when (get-text-property (point) 'w3m-href-anchor)
-        ;; return point when current is valid link
-        (throw 'reach nil))))
+    (let ((pos (point)))
+      (while (setq pos (previous-single-property-change pos 'w3m-anchor-sequence))
+        (when (get-text-property pos 'w3m-href-anchor)
+          ;; jump to previous anchor
+          (goto-char pos)
+          ;; return point when current is valid link
+          (throw 'reach nil)))))
   (point))

 (defun org-w3m-no-next-link-p ()
@@ -154,6 +190,7 @@ Return t if there is no next link; otherwise, return nil."
 (defun org-w3m-no-prev-link-p ()
   "Whether there is no previous link after the cursor.
 Return t if there is no previous link; otherwise, return nil."
+  ;; NOTE: This function seems never to be used. Should it be removed?
   (save-excursion
     (equal (point) (org-w3m-get-prev-link-start))))


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

* Re: bug#47088: org-w3m: handle w3m-image link information
  2021-05-06 22:05   ` Boruch Baum
@ 2021-05-06 23:56     ` Nick Savage
  2021-05-08  7:44       ` Bastien
  2021-05-09  6:50     ` Bastien
  1 sibling, 1 reply; 6+ messages in thread
From: Nick Savage @ 2021-05-06 23:56 UTC (permalink / raw)
  To: emacs-orgmode

I haven't opened up the code yet to take a look, but I likely will take 
a crack at some of these items in the future as low-hanging fruit 
refactoring project.

On 5/6/21 6:05 PM, Boruch Baum wrote:
> 	(org-w3m-get-image-end): New function, for w3m-img links.
>
> 3] As mentioned in the patch, and in the commit message, there seem to
>     be several functioned unused in the code base that could be
>     candidates for removal...
>
>          org-w3m-get-anchor-start
>          org-w3m-no-prev-link-p
>          org-w3m-get-prev-link-start
>
> 4] Additionally, in performing the merge I came across function
>     org-string-nw-p, which seems to be an unnecessary duplicate of the
>     emacs core function string-blank-p. It may be that historically you
>     once needed your own home-made functions/macros, but now that emacs
>     core has them, you may want to consider a refactor.


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

* Re: bug#47088: org-w3m: handle w3m-image link information
  2021-05-06 23:56     ` Nick Savage
@ 2021-05-08  7:44       ` Bastien
  2021-05-08 11:35         ` Nicholas Savage
  0 siblings, 1 reply; 6+ messages in thread
From: Bastien @ 2021-05-08  7:44 UTC (permalink / raw)
  To: Nick Savage; +Cc: emacs-orgmode

Hi Nick,

Nick Savage <nick@nicksavage.ca> writes:

> I haven't opened up the code yet to take a look, but I likely will
> take a crack at some of these items in the future as low-hanging fruit
> refactoring project.

Please feel free to go ahead and enhance ol-w3m.el in Org's repo with
the patches that Boruch kindly provided---and other enhancements that
you would find useful.

Thanks,

-- 
 Bastien


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

* Re: bug#47088: org-w3m: handle w3m-image link information
  2021-05-08  7:44       ` Bastien
@ 2021-05-08 11:35         ` Nicholas Savage
  0 siblings, 0 replies; 6+ messages in thread
From: Nicholas Savage @ 2021-05-08 11:35 UTC (permalink / raw)
  To: Emanuel Berg via General discussions about Org-mode.

I have applied it now as commit 5d2ccdae7f in master. Thank you, Boruch!

On Sat, May 8, 2021, at 03:44, Bastien wrote:
> Hi Nick,
> 
> Nick Savage <nick@nicksavage.ca> writes:
> 
> > I haven't opened up the code yet to take a look, but I likely will
> > take a crack at some of these items in the future as low-hanging fruit
> > refactoring project.
> 
> Please feel free to go ahead and enhance ol-w3m.el in Org's repo with
> the patches that Boruch kindly provided---and other enhancements that
> you would find useful.
> 
> Thanks,
> 
> -- 
>  Bastien
> 


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

* bug#47088: org-w3m: handle w3m-image link information
  2021-05-06 22:05   ` Boruch Baum
  2021-05-06 23:56     ` Nick Savage
@ 2021-05-09  6:50     ` Bastien
  1 sibling, 0 replies; 6+ messages in thread
From: Bastien @ 2021-05-09  6:50 UTC (permalink / raw)
  To: Boruch Baum; +Cc: 47088-done

I'm closing this report as the patch has been applied upstream.

-- 
 Bastien




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

end of thread, other threads:[~2021-05-09  6:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210312062614.usandeuwplmn7fu3@E15-2016.optimum.net>
2021-05-05  8:16 ` bug#47088: org-w3m: handle w3m-image link information Bastien
2021-05-06 22:05   ` Boruch Baum
2021-05-06 23:56     ` Nick Savage
2021-05-08  7:44       ` Bastien
2021-05-08 11:35         ` Nicholas Savage
2021-05-09  6:50     ` Bastien

Code repositories for project(s) associated with this 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).