Re: master 188bd80: gnus-shorten-url: Improve and avoid args-out-of-range error

Re: master 188bd80: gnus-shorten-url: Improve and avoid args-out-of-range error

[Stefan Monnier]

> +;;;###autoload
> +(defun string-truncate-left (string length)
> +  "Truncate STRING to LENGTH, replacing initial surplus with \"...\"."
> +  (let ((strlen (length string)))
> +    (if (<= strlen length)
> +	string
> +      (setq length (max 0 (- length 3)))
> +      (concat "..." (substring string (max 0 (- strlen 1 length)))))))

This should of course rely on string-width rather than string-length,
but more importantly, it should obey `truncate-string-ellipsis` and
it should be "closer" to `truncate-string-to-width` (they should likely
be in the same file, and with similar sounding names).
Maybe it should even be merged with `truncate-string-to-width`.


        Stefan

Re: master 188bd80: gnus-shorten-url: Improve and avoid args-out-of-range error

[Štěpán Němec]

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

On Mon, 13 Apr 2020 12:51:58 -0400
Stefan Monnier wrote:

>> +;;;###autoload
>> +(defun string-truncate-left (string length)
>> +  "Truncate STRING to LENGTH, replacing initial surplus with \"...\"."
>> +  (let ((strlen (length string)))
>> +    (if (<= strlen length)
>> +	string
>> +      (setq length (max 0 (- length 3)))
>> +      (concat "..." (substring string (max 0 (- strlen 1 length)))))))
>
> This should of course rely on string-width rather than string-length,
> but more importantly, it should obey `truncate-string-ellipsis` and
> it should be "closer" to `truncate-string-to-width` (they should likely
> be in the same file, and with similar sounding names).
> Maybe it should even be merged with `truncate-string-to-width`.

As the commit message says, that's really just a renamed helper function
originally used by ediff for file names (and now also in
`gnus-shorten-url').

Rewriting it to use `string-width' will require adjusting the callers,
too, but that's probably a good thing, as it should lead to more
correct results with strings containing wide characters. Still not
necessarily really correct results, though, as AFAICT the "columns"
which `string-width' speaks about are just an approximation, depending
on the fonts used.

E.g. with the default Chinese font emacs -Q uses on my system, I get
roughly 8.5 "columns" per 5 Chinese characters, not 10 as claimed by
`string-width'.

I also don't understand why the result of `string-width' should depend
on `current-language-environment', e.g. with "Chinese-GBK",
(string-width "…") returns 2 (why?!), with "English" or "UTF-8" it
returns 1, even though the display (font, "columns") stays the same for
all of them.

As for possible merging with `truncate-string-to-width', I don't think
I'm up to it; I was struggling to understand its doc string, let alone
the implementation.

Here's what I was able to come up with (BTW, I have little experience
with RTL scripts, but, doesn't in that case the ellipsis end up on the
logically wrong side, i.e. with the beginning/end of string reversed?):


[-- Attachment #2: 0001-string-truncate-left-Use-string-width-and-truncate-s.patch --]
[-- Type: text/x-patch, Size: 9177 bytes --]

From ad95727d0858767f14b27f412b12281a1a279870 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20N=C4=9Bmec?= <stepnem@gmail.com>
Date: Tue, 14 Apr 2020 11:08:50 +0200
Subject: [PATCH] string-truncate-left: Use string-width and
 truncate-string-ellipsis

https://lists.gnu.org/archive/html/emacs-devel/2020-04/msg00734.html
<jwvv9m3nxfa.fsf-monnier+emacs@gnu.org>

* lisp/emacs-lisp/subr-x.el (string-truncate-left): Rename and move...
* lisp/international/mule-util.el (truncate-string-left): ...here.
Use 'string-width' instead of 'string-length', respect
'truncate-string-ellipsis'.  All callers changed.
* lisp/gnus/gnus-sum.el (gnus-shorten-url): Use 'string-width'.
* test/lisp/international/mule-util-tests.el (truncate-string-left):
New test.
---
 lisp/emacs-lisp/subr-x.el                  |  9 ----
 lisp/gnus/gnus-sum.el                      |  7 ++--
 lisp/international/mule-util.el            | 13 ++++++
 lisp/vc/ediff-mult.el                      | 14 +++----
 test/lisp/international/mule-util-tests.el | 49 +++++++++++++++++++++-
 5 files changed, 71 insertions(+), 21 deletions(-)

diff --git a/lisp/emacs-lisp/subr-x.el b/lisp/emacs-lisp/subr-x.el
index 9f96ac50d1..044c9aada0 100644
--- a/lisp/emacs-lisp/subr-x.el
+++ b/lisp/emacs-lisp/subr-x.el
@@ -236,15 +236,6 @@ string-trim
 TRIM-LEFT and TRIM-RIGHT default to \"[ \\t\\n\\r]+\"."
   (string-trim-left (string-trim-right string trim-right) trim-left))
 
-;;;###autoload
-(defun string-truncate-left (string length)
-  "Truncate STRING to LENGTH, replacing initial surplus with \"...\"."
-  (let ((strlen (length string)))
-    (if (<= strlen length)
-	string
-      (setq length (max 0 (- length 3)))
-      (concat "..." (substring string (max 0 (- strlen 1 length)))))))
-
 (defsubst string-blank-p (string)
   "Check whether STRING is either empty or only whitespace.
 The following characters count as whitespace here: space, tab, newline and
diff --git a/lisp/gnus/gnus-sum.el b/lisp/gnus/gnus-sum.el
index 6f367692dd..2aa4e483c0 100644
--- a/lisp/gnus/gnus-sum.el
+++ b/lisp/gnus/gnus-sum.el
@@ -9494,15 +9494,16 @@ gnus-collect-urls
     (delete-dups urls)))
 
 (defun gnus-shorten-url (url max)
-  "Return an excerpt from URL not exceeding MAX characters."
-  (if (<= (length url) max)
+  "Return an excerpt from URL not exceeding MAX \"columns\".
+For the meaning of \"column\" see `truncate-string-to-width'."
+  (if (<= (string-width url) max)
       url
     (let* ((parsed (url-generic-parse-url url))
            (host (url-host parsed))
            (rest (concat (url-filename parsed)
                          (when-let ((target (url-target parsed)))
                            (concat "#" target)))))
-      (concat host (string-truncate-left rest (- max (length host)))))))
+      (concat host (truncate-string-left rest (- max (string-width host)))))))
 
 (defun gnus-summary-browse-url (&optional external)
   "Scan the current article body for links, and offer to browse them.
diff --git a/lisp/international/mule-util.el b/lisp/international/mule-util.el
index caa5747817..693601ea45 100644
--- a/lisp/international/mule-util.el
+++ b/lisp/international/mule-util.el
@@ -129,6 +129,19 @@ truncate-string-to-width
         (concat head-padding (substring str from-idx idx)
 	        tail-padding ellipsis)))))
 
+;;;###autoload
+(defun truncate-string-left (string width)
+  "Truncate STRING to WIDTH, replacing initial surplus with an ellipsis.
+The ellipsis used is the value of `truncate-string-ellipsis'."
+  (let ((strwidth (string-width string)))
+    (if (<= strwidth width)
+	string
+      (let ((ellipsis-width (string-width truncate-string-ellipsis)))
+        (if (>= ellipsis-width width)
+            (truncate-string-to-width string strwidth (- strwidth width))
+          (concat truncate-string-ellipsis
+                  (truncate-string-to-width
+                   string strwidth (+ (- strwidth width) ellipsis-width))))))))
 \f
 ;;; Nested alist handler.
 ;; Nested alist is alist whose elements are also nested alist.
diff --git a/lisp/vc/ediff-mult.el b/lisp/vc/ediff-mult.el
index 2b1b07927f..6a6a2da7b9 100644
--- a/lisp/vc/ediff-mult.el
+++ b/lisp/vc/ediff-mult.el
@@ -1171,7 +1171,7 @@ ediff-meta-insert-file-info1
 	  ;; abbreviate the file name, if file exists
 	  (if (and (not (stringp fname)) (< file-size -1))
 	      "-------"		; file doesn't exist
-	    (string-truncate-left
+	    (truncate-string-left
 	     (ediff-abbreviate-file-name fname)
 	     max-filename-width)))))))
 
@@ -1265,12 +1265,12 @@ ediff-draw-dir-diffs
 	(if (= (mod membership-code ediff-membership-code1) 0) ; dir1
 	    (let ((beg (point)))
 	      (insert (format "%-27s"
-			      (string-truncate-left
+			      (truncate-string-left
 			       (ediff-abbreviate-file-name
 				(if (file-directory-p (concat dir1 file))
 				    (file-name-as-directory file)
 				  file))
-			       24)))
+			       27)))
 	      ;; format of meta info in the dir-diff-buffer:
 	      ;;    (filename-tail filename-full otherdir1 otherdir2 otherdir3)
 	      (ediff-set-meta-overlay
@@ -1280,12 +1280,12 @@ ediff-draw-dir-diffs
 	(if (= (mod membership-code ediff-membership-code2) 0) ; dir2
 	    (let ((beg (point)))
 	      (insert (format "%-26s"
-			      (string-truncate-left
+			      (truncate-string-left
 			       (ediff-abbreviate-file-name
 				(if (file-directory-p (concat dir2 file))
 				    (file-name-as-directory file)
 				  file))
-			       24)))
+			       26)))
 	      (ediff-set-meta-overlay
 	       beg (point)
 	       (list meta-buf file (concat dir2 file) dir1 dir2 dir3)))
@@ -1294,12 +1294,12 @@ ediff-draw-dir-diffs
 	    (if (= (mod membership-code ediff-membership-code3) 0) ; dir3
 		(let ((beg (point)))
 		  (insert (format " %-25s"
-				  (string-truncate-left
+				  (truncate-string-left
 				   (ediff-abbreviate-file-name
 				    (if (file-directory-p (concat dir3 file))
 					(file-name-as-directory file)
 				      file))
-				   24)))
+				   25)))
 		  (ediff-set-meta-overlay
 		   beg (point)
 		   (list meta-buf file (concat dir3 file) dir1 dir2 dir3)))
diff --git a/test/lisp/international/mule-util-tests.el b/test/lisp/international/mule-util-tests.el
index c571782d63..403b355bb6 100644
--- a/test/lisp/international/mule-util-tests.el
+++ b/test/lisp/international/mule-util-tests.el
@@ -1,4 +1,4 @@
-;;; mule-util --- tests for international/mule-util.el
+;;; mule-util-tests --- tests for international/mule-util.el
 
 ;; Copyright (C) 2002-2020 Free Software Foundation, Inc.
 
@@ -81,4 +81,49 @@ mule-util-test-truncate-create
 (dotimes (i (length mule-util-test-truncate-data))
   (mule-util-test-truncate-create i))
 
-;;; mule-util.el ends here
+(ert-deftest truncate-string-left ()
+  (let ((truncate-string-ellipsis "..."))
+    (should (equal (truncate-string-left "ahojky jojky mojky" 10)
+                   "...y mojky"))
+    (should (equal (truncate-string-left "jojky mojky" 10)
+                   "...y mojky"))
+    (should (equal (truncate-string-left "jojky" 10)
+                   "jojky"))
+    (should (equal (truncate-string-left "jojky" 3)
+                   "jky"))
+    (should (equal (truncate-string-left "我的老田野" 10)
+                   "我的老田野"))
+    (should (equal (truncate-string-left "碩鼠碩鼠，甭食我叔" 10)
+                   "...食我叔"))
+    (should (equal (truncate-string-left "碩鼠碩鼠，甭食我叔" 3)
+                   "叔"))
+    (should (equal (truncate-string-left "碩鼠碩鼠，jojky" 10)
+                   "...，jojky")))
+  (let ((truncate-string-ellipsis "......"))
+    (should (equal (truncate-string-left "ahojky jojky mojky" 10)
+                   "......ojky"))
+    (should (equal (truncate-string-left "jojky" 3)
+                   "jky"))
+    (should (equal (truncate-string-left "我的老田野" 10)
+                   "我的老田野"))
+    (should (equal (truncate-string-left "碩鼠碩鼠，甭食我叔" 10)
+                   "......我叔"))
+    (should (equal (truncate-string-left "碩鼠碩鼠，甭食我叔" 3)
+                   "叔"))
+    (should (equal (truncate-string-left "碩鼠碩鼠，jojky" 10)
+                   "......ojky")))
+  (let ((truncate-string-ellipsis "…"))
+    (should (equal (truncate-string-left "ahojky jojky mojky" 10)
+                   "…jky mojky"))
+    (should (equal (truncate-string-left "jojky" 3)
+                   "…ky"))
+    (should (equal (truncate-string-left "我的老田野" 10)
+                   "我的老田野"))
+    (should (equal (truncate-string-left "碩鼠碩鼠，甭食我叔" 10)
+                   "…甭食我叔"))
+    (should (equal (truncate-string-left "碩鼠碩鼠，甭食我叔" 3)
+                   "…叔"))
+    (should (equal (truncate-string-left "碩鼠碩鼠，jojky" 10)
+                   "…鼠，jojky"))))
+
+;;; mule-util-tests.el ends here
-- 
2.26.0

Re: master 188bd80: gnus-shorten-url: Improve and avoid args-out-of-range error

[Eli Zaretskii]

> From: Štěpán Němec <stepnem@gmail.com>
> Date: Tue, 14 Apr 2020 11:26:22 +0200
> Cc: emacs-devel@gnu.org
> 
> Rewriting it to use `string-width' will require adjusting the callers,
> too, but that's probably a good thing, as it should lead to more
> correct results with strings containing wide characters. Still not
> necessarily really correct results, though, as AFAICT the "columns"
> which `string-width' speaks about are just an approximation, depending
> on the fonts used.
> 
> E.g. with the default Chinese font emacs -Q uses on my system, I get
> roughly 8.5 "columns" per 5 Chinese characters, not 10 as claimed by
> `string-width'.

AFAIK, string-width is exact only on TTY frames.

> I also don't understand why the result of `string-width' should depend
> on `current-language-environment', e.g. with "Chinese-GBK",
> (string-width "…") returns 2 (why?!), with "English" or "UTF-8" it
> returns 1, even though the display (font, "columns") stays the same for
> all of them.

See the definition of the Chinese-GBK language, it calls
use-cjk-char-width-table that sets up the char-width-table entries
specially, not sure why.  Maybe Handa-san can comment on this.

> (BTW, I have little experience with RTL scripts, but, doesn't in
> that case the ellipsis end up on the logically wrong side, i.e. with
> the beginning/end of string reversed?):

No.  The ellipsis should be _before_ the tail, in the reading order,
and that is what you get here.  If you want to dwell on this issue a
bit longer, look what the truncated string looks like in a buffer
whose bidi-paragraph-direction is set to right-to-left.

Re: master 188bd80: gnus-shorten-url: Improve and avoid args-out-of-range error

[Štěpán Němec]

On Tue, 14 Apr 2020 14:55:39 +0300
Eli Zaretskii wrote:

>> E.g. with the default Chinese font emacs -Q uses on my system, I get
>> roughly 8.5 "columns" per 5 Chinese characters, not 10 as claimed by
>> `string-width'.
>
> AFAIK, string-width is exact only on TTY frames.

Interesting, thanks. Given that I remember the issue (for graphical
frames) being discussed multiple times in the past, asking whether
making it work by default on those as well would be too hard, is
probably pointless?

> See the definition of the Chinese-GBK language, it calls
> use-cjk-char-width-table that sets up the char-width-table entries
> specially, not sure why.  Maybe Handa-san can comment on this.

Now I see that "…" causes other stange behaviour with Chinese-GBK on TTY
frames, too, probably as an artifact of its having "width" 2: it causes
cursor to skip the following character when moving over it.

>> (BTW, I have little experience with RTL scripts, but, doesn't in
>> that case the ellipsis end up on the logically wrong side, i.e. with
>> the beginning/end of string reversed?):
>
> No.  The ellipsis should be _before_ the tail, in the reading order,
> and that is what you get here.  If you want to dwell on this issue a
> bit longer, look what the truncated string looks like in a buffer
> whose bidi-paragraph-direction is set to right-to-left.

Thanks. I was mainly concerned about the function name: if that's the
case, it had better be called something like "truncate-string-beginning"
instead of "-left"?

-- 
Štěpán

Re: master 188bd80: gnus-shorten-url: Improve and avoid args-out-of-range error

[Eli Zaretskii]

> From: Štěpán Němec <stepnem@gmail.com>
> Cc: Kenichi Handa <handa@gnu.org>,  monnier@iro.umontreal.ca,
>   emacs-devel@gnu.org
> Date: Tue, 14 Apr 2020 14:24:28 +0200
> 
> On Tue, 14 Apr 2020 14:55:39 +0300
> Eli Zaretskii wrote:
> 
> >> E.g. with the default Chinese font emacs -Q uses on my system, I get
> >> roughly 8.5 "columns" per 5 Chinese characters, not 10 as claimed by
> >> `string-width'.
> >
> > AFAIK, string-width is exact only on TTY frames.
> 
> Interesting, thanks. Given that I remember the issue (for graphical
> frames) being discussed multiple times in the past, asking whether
> making it work by default on those as well would be too hard, is
> probably pointless?

I don't remember past discussions, but I guess the main difficulty
with making char-width mode accurate on GUI frames is that doing so
requires access to the font information, something that can be slow.
maybe we should have two separate APIs, the other one more accurate
but also potentially slower.

> > See the definition of the Chinese-GBK language, it calls
> > use-cjk-char-width-table that sets up the char-width-table entries
> > specially, not sure why.  Maybe Handa-san can comment on this.
> 
> Now I see that "…" causes other stange behaviour with Chinese-GBK on TTY
> frames, too, probably as an artifact of its having "width" 2: it causes
> cursor to skip the following character when moving over it.

I think the GBK setting assumes some specific fonts to be used.  Does
anything change if you start Emacs in a GBK locale to begin with?

> > No.  The ellipsis should be _before_ the tail, in the reading order,
> > and that is what you get here.  If you want to dwell on this issue a
> > bit longer, look what the truncated string looks like in a buffer
> > whose bidi-paragraph-direction is set to right-to-left.
> 
> Thanks. I was mainly concerned about the function name: if that's the
> case, it had better be called something like "truncate-string-beginning"
> instead of "-left"?

Yes, "-beginning" would be more accurate, I think.

Re: master 188bd80: gnus-shorten-url: Improve and avoid args-out-of-range error

[Štěpán Němec]

On Tue, 14 Apr 2020 15:42:55 +0300
Eli Zaretskii wrote:

> I don't remember past discussions,

They usually come in the form of "why are CJK misaligned in
tables/buffer list" or "how do I get wide characters exactly twice as
wide as non-wide characters, without messing up line height" etc.

https://lists.gnu.org/archive/cgi-bin/namazu.cgi?query=CJK+alignment&submit=Search%21&idxname=emacs-devel&max=100&result=normal&sort=date%3Alate

Maybe the second issue has more to do with the display engine, but I
thought more accurate `string-width' should help with the first one at
least.

>> > See the definition of the Chinese-GBK language, it calls
>> > use-cjk-char-width-table that sets up the char-width-table entries
>> > specially, not sure why.  Maybe Handa-san can comment on this.
>> 
>> Now I see that "…" causes other stange behaviour with Chinese-GBK on TTY
>> frames, too, probably as an artifact of its having "width" 2: it causes
>> cursor to skip the following character when moving over it.
>
> I think the GBK setting assumes some specific fonts to be used.  Does
> anything change if you start Emacs in a GBK locale to begin with?

I start emacs with LC_CTYPE="zh_CN.UTF-8" (which AFAIK is still the only
way to persuade it to use a system-wide input method), and in my init
file `set-language-environment' to "UTF-8". When I don't do the latter,
Emacs picks the "Chinese-GBK" environment. I only found that out
yesterday, being at a loss as to why my tests (involving "…") passed in
my usual Emacs and failed in emacs -Q (still started with the LC_CTYPE
set, though).

-- 
Štěpán

Re: master 188bd80: gnus-shorten-url: Improve and avoid args-out-of-range error

[Eli Zaretskii]

> From: Štěpán Němec <stepnem@gmail.com>
> Cc: handa@gnu.org,  monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Tue, 14 Apr 2020 15:48:19 +0200
> 
> On Tue, 14 Apr 2020 15:42:55 +0300
> Eli Zaretskii wrote:
> 
> > I don't remember past discussions,
> 
> They usually come in the form of "why are CJK misaligned in
> tables/buffer list" or "how do I get wide characters exactly twice as
> wide as non-wide characters, without messing up line height" etc.

I think alignment on GUI frames is a separate issue, because most
characters involved in this have glyphs that are neither 1 nor 2
columns, but somewhere in-between.  And the only way of aligning those
is by using the :align-to display spec.  Unless one succeeds in
finding a font that has each glyph's width to be an integral multiple
of the font size.

> > I think the GBK setting assumes some specific fonts to be used.  Does
> > anything change if you start Emacs in a GBK locale to begin with?
> 
> I start emacs with LC_CTYPE="zh_CN.UTF-8" (which AFAIK is still the only
> way to persuade it to use a system-wide input method), and in my init
> file `set-language-environment' to "UTF-8". When I don't do the latter,
> Emacs picks the "Chinese-GBK" environment.

I hoped that using a GBK locale would somehow change the fonts being
used, but maybe this no longer works on modern systems.  That part of
characters.el needs revisiting, the problem is that we no longer have
people available who remember why this was done.  Handa-san is my last
hope, as I think he wrote most of that code.

Re: master 188bd80: gnus-shorten-url: Improve and avoid args-out-of-range error

[Stefan Monnier]

> I don't remember past discussions, but I guess the main difficulty
> with making char-width mode accurate on GUI frames is that doing so
> requires access to the font information, something that can be slow.
> maybe we should have two separate APIs, the other one more accurate
> but also potentially slower.

I think it's worse than that: not only it can be slow but the
correct answer depends on face properties, frame properties, ...
A specific char in a specific position in a specific buffer can even
have different width at the same time if that buffer is displayed in
different frames.

AFAICT the only way to really "do it right" is to dump the work onto the
redisplay, e.g. adding the ability (e.g. via `display` properties) to
label a chunk of text as "right-align", or as "truncate to <WIDTH>", or
...


        Stefan