unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#33043: 27.0.50; wishlist: rcirc should allow truncation of visible URLs
@ 2018-10-15 10:04 David Edmondson
  2018-10-15 10:06 ` bug#33043: [PATCH v1 1/1] Support for the truncation of displayed URLs David Edmondson
  0 siblings, 1 reply; 11+ messages in thread
From: David Edmondson @ 2018-10-15 10:04 UTC (permalink / raw)
  To: 33043

With the use of IRC as a gateway to other protocols (Mastodon, Twitter,
...) it is very common to see long URLs in rcirc buffers. It would be
nice if the visible part of these could be truncated to improve their
display.

Proposed patch in followup...

dme.
-- 
The band is just fantastic, that is really what I think.





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

* bug#33043: [PATCH v1 1/1] Support for the truncation of displayed URLs
  2018-10-15 10:04 bug#33043: 27.0.50; wishlist: rcirc should allow truncation of visible URLs David Edmondson
@ 2018-10-15 10:06 ` David Edmondson
  2018-10-17 22:27   ` Basil L. Contovounesios
  0 siblings, 1 reply; 11+ messages in thread
From: David Edmondson @ 2018-10-15 10:06 UTC (permalink / raw)
  To: 33043

When applying markup to URLs in the buffer, truncate the visible
portion to `rcirc-url-max-length' characters. By default no truncation
is done.
---
 lisp/net/rcirc.el | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
index fe9c71a21c..1c0b540563 100644
--- a/lisp/net/rcirc.el
+++ b/lisp/net/rcirc.el
@@ -168,6 +168,13 @@ rcirc-fill-prefix
 		 (string :tag "Prefix text"))
   :group 'rcirc)
 
+(defcustom rcirc-url-max-length nil
+  "Maximum number of characters in displayed URLs.
+If nil, no maximum is applied."
+  :type '(choice (const :tag "No maximum" nil)
+                 (integer :tag "Number of characters"))
+  :group 'rcirc)
+
 (defvar rcirc-ignore-buffer-activity-flag nil
   "If non-nil, ignore activity in this buffer.")
 (make-variable-buffer-local 'rcirc-ignore-buffer-activity-flag)
@@ -2491,6 +2498,16 @@ rcirc-markup-urls
            (end (match-end 0))
            (url (match-string-no-properties 0))
            (link-text (buffer-substring-no-properties start end)))
+      ;; Truncate the visible part of URLs if required and necessary.
+      (when (and rcirc-url-max-length
+		 (> (- end start) rcirc-url-max-length))
+	(let* ((ellipsis "...")
+               (new-end (- (+ start rcirc-url-max-length)
+                           (length ellipsis))))
+	  (delete-region new-end end)
+	  (goto-char new-end)
+	  (insert ellipsis)
+	  (setq end (point))))
       ;; Add a button for the URL.  Note that we use `make-text-button',
       ;; rather than `make-button', as text-buttons are much faster in
       ;; large buffers.
-- 
2.19.1






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

* bug#33043: [PATCH v1 1/1] Support for the truncation of displayed URLs
  2018-10-15 10:06 ` bug#33043: [PATCH v1 1/1] Support for the truncation of displayed URLs David Edmondson
@ 2018-10-17 22:27   ` Basil L. Contovounesios
  2018-10-18  9:39     ` David Edmondson
  0 siblings, 1 reply; 11+ messages in thread
From: Basil L. Contovounesios @ 2018-10-17 22:27 UTC (permalink / raw)
  To: David Edmondson; +Cc: 33043

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

Thanks for working on this David, I like the idea.

David Edmondson <dme@dme.org> writes:

> @@ -2491,6 +2498,16 @@ rcirc-markup-urls
>             (end (match-end 0))
>             (url (match-string-no-properties 0))
>             (link-text (buffer-substring-no-properties start end)))
> +      ;; Truncate the visible part of URLs if required and necessary.
> +      (when (and rcirc-url-max-length
> +		 (> (- end start) rcirc-url-max-length))
> +	(let* ((ellipsis "...")
> +               (new-end (- (+ start rcirc-url-max-length)
> +                           (length ellipsis))))
> +	  (delete-region new-end end)
> +	  (goto-char new-end)
> +	  (insert ellipsis)
> +	  (setq end (point))))
>        ;; Add a button for the URL.  Note that we use `make-text-button',
>        ;; rather than `make-button', as text-buttons are much faster in
>        ;; large buffers.

Couldn't we reuse url-truncate-url-for-viewing, which tries to be smart
about where it places the ellipsis, for this?  I think the existing code
can also be cleaned up a little:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: rcirc.diff --]
[-- Type: text/x-diff, Size: 2014 bytes --]

diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
index fe9c71a21c..de28cdb054 100644
--- a/lisp/net/rcirc.el
+++ b/lisp/net/rcirc.el
@@ -2485,24 +2485,25 @@ rcirc-markup-my-nick
 	(rcirc-record-activity (current-buffer) 'nick)))))
 
 (defun rcirc-markup-urls (_sender _response)
-  (while (and rcirc-url-regexp ;; nil means disable URL catching
+  (while (and rcirc-url-regexp ; nil means disable URL catching.
               (re-search-forward rcirc-url-regexp nil t))
-    (let* ((start (match-beginning 0))
+    (let* ((beg (match-beginning 0))
            (end (match-end 0))
-           (url (match-string-no-properties 0))
-           (link-text (buffer-substring-no-properties start end)))
-      ;; Add a button for the URL.  Note that we use `make-text-button',
-      ;; rather than `make-button', as text-buttons are much faster in
-      ;; large buffers.
-      (make-text-button start end
-			'face 'rcirc-url
-			'follow-link t
-			'rcirc-url url
-			'action (lambda (button)
-				  (browse-url (button-get button 'rcirc-url))))
-      ;; record the url if it is not already the latest stored url
-      (when (not (string= link-text (caar rcirc-urls)))
-        (push (cons link-text start) rcirc-urls)))))
+           (url (buffer-substring-no-properties beg end)))
+      (delete-region beg end)
+      ;; Add a button for the URL.  Note that we use
+      ;; `insert-text-button', rather than `insert-button', as text
+      ;; property buttons are much faster in large buffers.
+      (insert-text-button
+       (url-truncate-url-for-viewing url rcirc-url-max-length)
+       'face 'rcirc-url
+       'follow-link t
+       'rcirc-url url
+       'action (lambda (button)
+                 (browse-url (button-get button 'rcirc-url))))
+      ;; Record the URL if it is not already the latest stored URL.
+      (unless (string= url (caar rcirc-urls))
+        (push (cons url beg) rcirc-urls)))))
 
 (defun rcirc-markup-keywords (sender response)
   (when (and (string= response "PRIVMSG")

[-- Attachment #3: Type: text/plain, Size: 18 bytes --]


WDYT?

-- 
Basil

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

* bug#33043: [PATCH v1 1/1] Support for the truncation of displayed URLs
  2018-10-17 22:27   ` Basil L. Contovounesios
@ 2018-10-18  9:39     ` David Edmondson
  2018-10-18 10:38       ` Basil L. Contovounesios
  0 siblings, 1 reply; 11+ messages in thread
From: David Edmondson @ 2018-10-18  9:39 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: 33043

On Wednesday, 2018-10-17 at 23:27:45 +01, Basil L. Contovounesios wrote:

> Couldn't we reuse url-truncate-url-for-viewing, which tries to be smart
> about where it places the ellipsis, for this?  I think the existing code
> can also be cleaned up a little:

I didn't know about url-truncate-url-for-viewing, no other reason.

I'd be happy with your change, but note that it will always truncate
URLs that are wider than the current frame - there is no way to have no
truncation take place.

If a user is most familiar with something like ffap to open URLs then
this is unfortunate - ffap doesn't see the full URL in the buffer.

> diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
> index fe9c71a21c..de28cdb054 100644
> --- a/lisp/net/rcirc.el
> +++ b/lisp/net/rcirc.el
> @@ -2485,24 +2485,25 @@ rcirc-markup-my-nick
>  	(rcirc-record-activity (current-buffer) 'nick)))))
>  
>  (defun rcirc-markup-urls (_sender _response)
> -  (while (and rcirc-url-regexp ;; nil means disable URL catching
> +  (while (and rcirc-url-regexp ; nil means disable URL catching.
>                (re-search-forward rcirc-url-regexp nil t))
> -    (let* ((start (match-beginning 0))
> +    (let* ((beg (match-beginning 0))
>             (end (match-end 0))
> -           (url (match-string-no-properties 0))
> -           (link-text (buffer-substring-no-properties start end)))
> -      ;; Add a button for the URL.  Note that we use `make-text-button',
> -      ;; rather than `make-button', as text-buttons are much faster in
> -      ;; large buffers.
> -      (make-text-button start end
> -			'face 'rcirc-url
> -			'follow-link t
> -			'rcirc-url url
> -			'action (lambda (button)
> -				  (browse-url (button-get button 'rcirc-url))))
> -      ;; record the url if it is not already the latest stored url
> -      (when (not (string= link-text (caar rcirc-urls)))
> -        (push (cons link-text start) rcirc-urls)))))
> +           (url (buffer-substring-no-properties beg end)))
> +      (delete-region beg end)
> +      ;; Add a button for the URL.  Note that we use
> +      ;; `insert-text-button', rather than `insert-button', as text
> +      ;; property buttons are much faster in large buffers.
> +      (insert-text-button
> +       (url-truncate-url-for-viewing url rcirc-url-max-length)
> +       'face 'rcirc-url
> +       'follow-link t
> +       'rcirc-url url
> +       'action (lambda (button)
> +                 (browse-url (button-get button 'rcirc-url))))
> +      ;; Record the URL if it is not already the latest stored URL.
> +      (unless (string= url (caar rcirc-urls))
> +        (push (cons url beg) rcirc-urls)))))
>  
>  (defun rcirc-markup-keywords (sender response)
>    (when (and (string= response "PRIVMSG")
>
> WDYT?
>
> -- 
> Basil

dme.
-- 
I've still got sand in my shoes.





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

* bug#33043: [PATCH v1 1/1] Support for the truncation of displayed URLs
  2018-10-18  9:39     ` David Edmondson
@ 2018-10-18 10:38       ` Basil L. Contovounesios
  2018-10-18 10:50         ` David Edmondson
  2018-10-27  8:54         ` Eli Zaretskii
  0 siblings, 2 replies; 11+ messages in thread
From: Basil L. Contovounesios @ 2018-10-18 10:38 UTC (permalink / raw)
  To: David Edmondson; +Cc: 33043

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

David Edmondson <dme@dme.org> writes:

> On Wednesday, 2018-10-17 at 23:27:45 +01, Basil L. Contovounesios wrote:
>
>> Couldn't we reuse url-truncate-url-for-viewing, which tries to be smart
>> about where it places the ellipsis, for this?  I think the existing code
>> can also be cleaned up a little:
>
> I didn't know about url-truncate-url-for-viewing, no other reason.
>
> I'd be happy with your change, but note that it will always truncate
> URLs that are wider than the current frame - there is no way to have no
> truncation take place.
>
> If a user is most familiar with something like ffap to open URLs then
> this is unfortunate - ffap doesn't see the full URL in the buffer.

Sorry, that was an oversight.  Here's how the last patch can be patched
to support the default of no truncation:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: rcirc.diff --]
[-- Type: text/x-diff, Size: 583 bytes --]

diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
index de28cdb054..31f0baddc0 100644
--- a/lisp/net/rcirc.el
+++ b/lisp/net/rcirc.el
@@ -2495,7 +2495,9 @@ rcirc-markup-urls
       ;; `insert-text-button', rather than `insert-button', as text
       ;; property buttons are much faster in large buffers.
       (insert-text-button
-       (url-truncate-url-for-viewing url rcirc-url-max-length)
+       (if rcirc-url-max-length
+           (url-truncate-url-for-viewing url rcirc-url-max-length)
+         url)
        'face 'rcirc-url
        'follow-link t
        'rcirc-url url

[-- Attachment #3: Type: text/plain, Size: 116 bytes --]


If you prefer to avoid deletion and insertion when rcirc-url-max-length
is nil, you can also write the following:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: rcirc-alt.diff --]
[-- Type: text/x-diff, Size: 1745 bytes --]

diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
index fe9c71a21c..4d62b382a6 100644
--- a/lisp/net/rcirc.el
+++ b/lisp/net/rcirc.el
@@ -2485,24 +2485,27 @@ rcirc-markup-my-nick
 	(rcirc-record-activity (current-buffer) 'nick)))))
 
 (defun rcirc-markup-urls (_sender _response)
-  (while (and rcirc-url-regexp ;; nil means disable URL catching
+  (while (and rcirc-url-regexp ; nil means disable URL catching.
               (re-search-forward rcirc-url-regexp nil t))
-    (let* ((start (match-beginning 0))
+    (let* ((beg (match-beginning 0))
            (end (match-end 0))
-           (url (match-string-no-properties 0))
-           (link-text (buffer-substring-no-properties start end)))
+           (url (buffer-substring-no-properties beg end)))
+      (when rcirc-url-max-length
+        (replace-match (url-truncate-url-for-viewing url rcirc-url-max-length)
+                       t t)
+        (setq end (point)))
       ;; Add a button for the URL.  Note that we use `make-text-button',
       ;; rather than `make-button', as text-buttons are much faster in
       ;; large buffers.
-      (make-text-button start end
+      (make-text-button beg end
 			'face 'rcirc-url
 			'follow-link t
 			'rcirc-url url
 			'action (lambda (button)
 				  (browse-url (button-get button 'rcirc-url))))
-      ;; record the url if it is not already the latest stored url
-      (when (not (string= link-text (caar rcirc-urls)))
-        (push (cons link-text start) rcirc-urls)))))
+      ;; Record the URL if it is not already the latest stored URL.
+      (unless (string= url (caar rcirc-urls))
+        (push (cons url beg) rcirc-urls)))))
 
 (defun rcirc-markup-keywords (sender response)
   (when (and (string= response "PRIVMSG")

[-- Attachment #5: Type: text/plain, Size: 20 bytes --]


Thanks,

-- 
Basil

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

* bug#33043: [PATCH v1 1/1] Support for the truncation of displayed URLs
  2018-10-18 10:38       ` Basil L. Contovounesios
@ 2018-10-18 10:50         ` David Edmondson
  2018-10-27  8:54         ` Eli Zaretskii
  1 sibling, 0 replies; 11+ messages in thread
From: David Edmondson @ 2018-10-18 10:50 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: 33043

On Thursday, 2018-10-18 at 11:38:04 +0100, Basil L. Contovounesios wrote:

> David Edmondson <dme@dme.org> writes:
>
>> On Wednesday, 2018-10-17 at 23:27:45 +01, Basil L. Contovounesios wrote:
>>
>>> Couldn't we reuse url-truncate-url-for-viewing, which tries to be smart
>>> about where it places the ellipsis, for this?  I think the existing code
>>> can also be cleaned up a little:
>>
>> I didn't know about url-truncate-url-for-viewing, no other reason.
>>
>> I'd be happy with your change, but note that it will always truncate
>> URLs that are wider than the current frame - there is no way to have no
>> truncation take place.
>>
>> If a user is most familiar with something like ffap to open URLs then
>> this is unfortunate - ffap doesn't see the full URL in the buffer.
>
> Sorry, that was an oversight.  Here's how the last patch can be patched
> to support the default of no truncation:

Either of your versions looks fine to me, thanks for looking into it.

> diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
> index de28cdb054..31f0baddc0 100644
> --- a/lisp/net/rcirc.el
> +++ b/lisp/net/rcirc.el
> @@ -2495,7 +2495,9 @@ rcirc-markup-urls
>        ;; `insert-text-button', rather than `insert-button', as text
>        ;; property buttons are much faster in large buffers.
>        (insert-text-button
> -       (url-truncate-url-for-viewing url rcirc-url-max-length)
> +       (if rcirc-url-max-length
> +           (url-truncate-url-for-viewing url rcirc-url-max-length)
> +         url)
>         'face 'rcirc-url
>         'follow-link t
>         'rcirc-url url
>
> If you prefer to avoid deletion and insertion when rcirc-url-max-length
> is nil, you can also write the following:
>
> diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
> index fe9c71a21c..4d62b382a6 100644
> --- a/lisp/net/rcirc.el
> +++ b/lisp/net/rcirc.el
> @@ -2485,24 +2485,27 @@ rcirc-markup-my-nick
>  	(rcirc-record-activity (current-buffer) 'nick)))))
>  
>  (defun rcirc-markup-urls (_sender _response)
> -  (while (and rcirc-url-regexp ;; nil means disable URL catching
> +  (while (and rcirc-url-regexp ; nil means disable URL catching.
>                (re-search-forward rcirc-url-regexp nil t))
> -    (let* ((start (match-beginning 0))
> +    (let* ((beg (match-beginning 0))
>             (end (match-end 0))
> -           (url (match-string-no-properties 0))
> -           (link-text (buffer-substring-no-properties start end)))
> +           (url (buffer-substring-no-properties beg end)))
> +      (when rcirc-url-max-length
> +        (replace-match (url-truncate-url-for-viewing url rcirc-url-max-length)
> +                       t t)
> +        (setq end (point)))
>        ;; Add a button for the URL.  Note that we use `make-text-button',
>        ;; rather than `make-button', as text-buttons are much faster in
>        ;; large buffers.
> -      (make-text-button start end
> +      (make-text-button beg end
>  			'face 'rcirc-url
>  			'follow-link t
>  			'rcirc-url url
>  			'action (lambda (button)
>  				  (browse-url (button-get button 'rcirc-url))))
> -      ;; record the url if it is not already the latest stored url
> -      (when (not (string= link-text (caar rcirc-urls)))
> -        (push (cons link-text start) rcirc-urls)))))
> +      ;; Record the URL if it is not already the latest stored URL.
> +      (unless (string= url (caar rcirc-urls))
> +        (push (cons url beg) rcirc-urls)))))
>  
>  (defun rcirc-markup-keywords (sender response)
>    (when (and (string= response "PRIVMSG")
>
> Thanks,
>
> -- 
> Basil

dme.
-- 
Does everyone stare the way I do?





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

* bug#33043: [PATCH v1 1/1] Support for the truncation of displayed URLs
  2018-10-18 10:38       ` Basil L. Contovounesios
  2018-10-18 10:50         ` David Edmondson
@ 2018-10-27  8:54         ` Eli Zaretskii
  2018-10-28  3:14           ` Basil L. Contovounesios
  1 sibling, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2018-10-27  8:54 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: 33043, dme

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Date: Thu, 18 Oct 2018 11:38:04 +0100
> Cc: 33043@debbugs.gnu.org
> 
> > I didn't know about url-truncate-url-for-viewing, no other reason.
> >
> > I'd be happy with your change, but note that it will always truncate
> > URLs that are wider than the current frame - there is no way to have no
> > truncation take place.
> >
> > If a user is most familiar with something like ffap to open URLs then
> > this is unfortunate - ffap doesn't see the full URL in the buffer.
> 
> Sorry, that was an oversight.  Here's how the last patch can be patched
> to support the default of no truncation:

If you two are in agreement wrt what should be installed, I'd need a
full changeset from Basil to push.

Thanks.





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

* bug#33043: [PATCH v1 1/1] Support for the truncation of displayed URLs
  2018-10-27  8:54         ` Eli Zaretskii
@ 2018-10-28  3:14           ` Basil L. Contovounesios
  2018-10-29 16:01             ` David Edmondson
  0 siblings, 1 reply; 11+ messages in thread
From: Basil L. Contovounesios @ 2018-10-28  3:14 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 33043, dme

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0001-Add-URL-truncation-support-to-rcirc-bug-33043.patch --]
[-- Type: text/x-diff, Size: 1912 bytes --]

From 76dc7ed5d5b9c6273e99a2659ba304e9e7eb8e03 Mon Sep 17 00:00:00 2001
From: David Edmondson <dme@dme.org>
Date: Sun, 28 Oct 2018 03:11:21 +0000
Subject: [PATCH 1/2] Add URL truncation support to rcirc (bug#33043)

* lisp/net/rcirc.el (rcirc-url-max-length): New user option
controlling extent of URL truncation, defaulting to none.
(rcirc-markup-urls): Use it.
---
 lisp/net/rcirc.el | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
index fe9c71a21c..50e4d8ffb2 100644
--- a/lisp/net/rcirc.el
+++ b/lisp/net/rcirc.el
@@ -168,6 +168,14 @@ rcirc-fill-prefix
 		 (string :tag "Prefix text"))
   :group 'rcirc)
 
+(defcustom rcirc-url-max-length nil
+  "Maximum number of characters in displayed URLs.
+If nil, no maximum is applied."
+  :version "27.1"
+  :type '(choice (const :tag "No maximum" nil)
+                 (integer :tag "Number of characters"))
+  :group 'rcirc)
+
 (defvar rcirc-ignore-buffer-activity-flag nil
   "If non-nil, ignore activity in this buffer.")
 (make-variable-buffer-local 'rcirc-ignore-buffer-activity-flag)
@@ -2491,6 +2499,16 @@ rcirc-markup-urls
            (end (match-end 0))
            (url (match-string-no-properties 0))
            (link-text (buffer-substring-no-properties start end)))
+      ;; Truncate the visible part of URLs if required and necessary.
+      (when (and rcirc-url-max-length
+                 (> (- end start) rcirc-url-max-length))
+        (let* ((ellipsis "...")
+               (new-end (- (+ start rcirc-url-max-length)
+                           (length ellipsis))))
+          (delete-region new-end end)
+          (goto-char new-end)
+          (insert ellipsis)
+          (setq end (point))))
       ;; Add a button for the URL.  Note that we use `make-text-button',
       ;; rather than `make-button', as text-buttons are much faster in
       ;; large buffers.
-- 
2.19.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Reuse-URL-truncation-function-in-rcirc-bug-33043.patch --]
[-- Type: text/x-diff, Size: 2624 bytes --]

From 8d7f03b12ccb07cd9ffe953b99d519e289b61df9 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Sun, 28 Oct 2018 02:51:18 +0000
Subject: [PATCH 2/2] Reuse URL truncation function in rcirc (bug#33043)

* lisp/net/rcirc.el (rcirc-markup-urls): Simplify using
url-truncate-url-for-viewing.
---
 lisp/net/rcirc.el | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
index 50e4d8ffb2..ca707341be 100644
--- a/lisp/net/rcirc.el
+++ b/lisp/net/rcirc.el
@@ -2493,34 +2493,26 @@ rcirc-markup-my-nick
 	(rcirc-record-activity (current-buffer) 'nick)))))
 
 (defun rcirc-markup-urls (_sender _response)
-  (while (and rcirc-url-regexp ;; nil means disable URL catching
+  (while (and rcirc-url-regexp ; nil means disable URL catching.
               (re-search-forward rcirc-url-regexp nil t))
     (let* ((start (match-beginning 0))
-           (end (match-end 0))
-           (url (match-string-no-properties 0))
-           (link-text (buffer-substring-no-properties start end)))
-      ;; Truncate the visible part of URLs if required and necessary.
-      (when (and rcirc-url-max-length
-                 (> (- end start) rcirc-url-max-length))
-        (let* ((ellipsis "...")
-               (new-end (- (+ start rcirc-url-max-length)
-                           (length ellipsis))))
-          (delete-region new-end end)
-          (goto-char new-end)
-          (insert ellipsis)
-          (setq end (point))))
+           (url   (buffer-substring-no-properties start (point))))
+      (when rcirc-url-max-length
+        ;; Replace match with truncated URL.
+        (delete-region start (point))
+        (insert (url-truncate-url-for-viewing url rcirc-url-max-length)))
       ;; Add a button for the URL.  Note that we use `make-text-button',
       ;; rather than `make-button', as text-buttons are much faster in
       ;; large buffers.
-      (make-text-button start end
+      (make-text-button start (point)
 			'face 'rcirc-url
 			'follow-link t
 			'rcirc-url url
 			'action (lambda (button)
 				  (browse-url (button-get button 'rcirc-url))))
-      ;; record the url if it is not already the latest stored url
-      (when (not (string= link-text (caar rcirc-urls)))
-        (push (cons link-text start) rcirc-urls)))))
+      ;; Record the URL if it is not already the latest stored URL.
+      (unless (string= url (caar rcirc-urls))
+        (push (cons url start) rcirc-urls)))))
 
 (defun rcirc-markup-keywords (sender response)
   (when (and (string= response "PRIVMSG")
-- 
2.19.1


[-- Attachment #3: Type: text/plain, Size: 1024 bytes --]


Eli Zaretskii <eliz@gnu.org> writes:

>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>> Date: Thu, 18 Oct 2018 11:38:04 +0100
>> Cc: 33043@debbugs.gnu.org
>> 
>> > I didn't know about url-truncate-url-for-viewing, no other reason.
>> >
>> > I'd be happy with your change, but note that it will always truncate
>> > URLs that are wider than the current frame - there is no way to have no
>> > truncation take place.
>> >
>> > If a user is most familiar with something like ffap to open URLs then
>> > this is unfortunate - ffap doesn't see the full URL in the buffer.
>> 
>> Sorry, that was an oversight.  Here's how the last patch can be patched
>> to support the default of no truncation:
>
> If you two are in agreement wrt what should be installed, I'd need a
> full changeset from Basil to push.

I attach two patches.  The first comprises David's original patch, but
with an added :version tag and reworded commit message.  The second
switches to reusing url-truncate-url-for-viewing.  WDYT?

Thanks,

-- 
Basil

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

* bug#33043: [PATCH v1 1/1] Support for the truncation of displayed URLs
  2018-10-28  3:14           ` Basil L. Contovounesios
@ 2018-10-29 16:01             ` David Edmondson
  2018-10-29 18:27               ` Basil L. Contovounesios
  0 siblings, 1 reply; 11+ messages in thread
From: David Edmondson @ 2018-10-29 16:01 UTC (permalink / raw)
  To: Basil L. Contovounesios, Eli Zaretskii; +Cc: 33043

On Sunday, 2018-10-28 at 03:14:39 +00, Basil L. Contovounesios wrote:

> I attach two patches.  The first comprises David's original patch, but
> with an added :version tag and reworded commit message.  The second
> switches to reusing url-truncate-url-for-viewing.  WDYT?

It would be simpler just to smash them together. I don't care about the
credit or anything.

dme.
-- 
Hello? Is anybody home? Well, you don't know me, but I know you.





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

* bug#33043: [PATCH v1 1/1] Support for the truncation of displayed URLs
  2018-10-29 16:01             ` David Edmondson
@ 2018-10-29 18:27               ` Basil L. Contovounesios
  2018-11-03  9:11                 ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Basil L. Contovounesios @ 2018-10-29 18:27 UTC (permalink / raw)
  To: David Edmondson; +Cc: 33043

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0001-Add-URL-truncation-support-to-rcirc-bug-33043.patch --]
[-- Type: text/x-diff, Size: 3407 bytes --]

From 41d7f52984e6220123b8ccc7da1b1d56a9a7eb49 Mon Sep 17 00:00:00 2001
From: David Edmondson <dme@dme.org>
Date: Sun, 28 Oct 2018 03:11:21 +0000
Subject: [PATCH] Add URL truncation support to rcirc (bug#33043)

Suggested by David Edmondson <dme@dme.org>.
* lisp/net/rcirc.el (rcirc-url-max-length): New user option
controlling extent of URL truncation, defaulting to none.
(rcirc-markup-urls): Use it.
* etc/NEWS: Announce it.
---
 etc/NEWS          |  7 +++++++
 lisp/net/rcirc.el | 26 ++++++++++++++++++--------
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 226ae1e135..f7fdd0add9 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -758,6 +758,13 @@ Tramp for some look-alike remote file names.
 *** For some connection methods, like "su" or "sudo", the host name in
 ad-hoc multi-hop file names must match the previous hop.
 
+** Rcirc
+
+---
+*** New user option 'rcirc-url-max-length'.
+Setting this option to an integer causes URLs displayed in Rcirc
+buffers to be truncated to that many characters.
+
 ** Register
 ---
 *** The return value of method 'register-val-describe' includes the
diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
index fe9c71a21c..ca707341be 100644
--- a/lisp/net/rcirc.el
+++ b/lisp/net/rcirc.el
@@ -168,6 +168,14 @@ rcirc-fill-prefix
 		 (string :tag "Prefix text"))
   :group 'rcirc)
 
+(defcustom rcirc-url-max-length nil
+  "Maximum number of characters in displayed URLs.
+If nil, no maximum is applied."
+  :version "27.1"
+  :type '(choice (const :tag "No maximum" nil)
+                 (integer :tag "Number of characters"))
+  :group 'rcirc)
+
 (defvar rcirc-ignore-buffer-activity-flag nil
   "If non-nil, ignore activity in this buffer.")
 (make-variable-buffer-local 'rcirc-ignore-buffer-activity-flag)
@@ -2485,24 +2493,26 @@ rcirc-markup-my-nick
 	(rcirc-record-activity (current-buffer) 'nick)))))
 
 (defun rcirc-markup-urls (_sender _response)
-  (while (and rcirc-url-regexp ;; nil means disable URL catching
+  (while (and rcirc-url-regexp ; nil means disable URL catching.
               (re-search-forward rcirc-url-regexp nil t))
     (let* ((start (match-beginning 0))
-           (end (match-end 0))
-           (url (match-string-no-properties 0))
-           (link-text (buffer-substring-no-properties start end)))
+           (url   (buffer-substring-no-properties start (point))))
+      (when rcirc-url-max-length
+        ;; Replace match with truncated URL.
+        (delete-region start (point))
+        (insert (url-truncate-url-for-viewing url rcirc-url-max-length)))
       ;; Add a button for the URL.  Note that we use `make-text-button',
       ;; rather than `make-button', as text-buttons are much faster in
       ;; large buffers.
-      (make-text-button start end
+      (make-text-button start (point)
 			'face 'rcirc-url
 			'follow-link t
 			'rcirc-url url
 			'action (lambda (button)
 				  (browse-url (button-get button 'rcirc-url))))
-      ;; record the url if it is not already the latest stored url
-      (when (not (string= link-text (caar rcirc-urls)))
-        (push (cons link-text start) rcirc-urls)))))
+      ;; Record the URL if it is not already the latest stored URL.
+      (unless (string= url (caar rcirc-urls))
+        (push (cons url start) rcirc-urls)))))
 
 (defun rcirc-markup-keywords (sender response)
   (when (and (string= response "PRIVMSG")
-- 
2.19.1


[-- Attachment #2: Type: text/plain, Size: 543 bytes --]


David Edmondson <dme@dme.org> writes:

> On Sunday, 2018-10-28 at 03:14:39 +00, Basil L. Contovounesios wrote:
>
>> I attach two patches.  The first comprises David's original patch, but
>> with an added :version tag and reworded commit message.  The second
>> switches to reusing url-truncate-url-for-viewing.  WDYT?
>
> It would be simpler just to smash them together. I don't care about the
> credit or anything.

I don't either.  How's the attached patch, which additionally announces
the new user option in etc/NEWS?

Thanks,

-- 
Basil

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

* bug#33043: [PATCH v1 1/1] Support for the truncation of displayed URLs
  2018-10-29 18:27               ` Basil L. Contovounesios
@ 2018-11-03  9:11                 ` Eli Zaretskii
  0 siblings, 0 replies; 11+ messages in thread
From: Eli Zaretskii @ 2018-11-03  9:11 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: 33043-done, dme

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: Eli Zaretskii <eliz@gnu.org>,  33043@debbugs.gnu.org
> Date: Mon, 29 Oct 2018 18:27:54 +0000
> 
> David Edmondson <dme@dme.org> writes:
> 
> > On Sunday, 2018-10-28 at 03:14:39 +00, Basil L. Contovounesios wrote:
> >
> >> I attach two patches.  The first comprises David's original patch, but
> >> with an added :version tag and reworded commit message.  The second
> >> switches to reusing url-truncate-url-for-viewing.  WDYT?
> >
> > It would be simpler just to smash them together. I don't care about the
> > credit or anything.
> 
> I don't either.  How's the attached patch, which additionally announces
> the new user option in etc/NEWS?

Thanks, pushed to the master branch.





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

end of thread, other threads:[~2018-11-03  9:11 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-15 10:04 bug#33043: 27.0.50; wishlist: rcirc should allow truncation of visible URLs David Edmondson
2018-10-15 10:06 ` bug#33043: [PATCH v1 1/1] Support for the truncation of displayed URLs David Edmondson
2018-10-17 22:27   ` Basil L. Contovounesios
2018-10-18  9:39     ` David Edmondson
2018-10-18 10:38       ` Basil L. Contovounesios
2018-10-18 10:50         ` David Edmondson
2018-10-27  8:54         ` Eli Zaretskii
2018-10-28  3:14           ` Basil L. Contovounesios
2018-10-29 16:01             ` David Edmondson
2018-10-29 18:27               ` Basil L. Contovounesios
2018-11-03  9:11                 ` Eli Zaretskii

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