bug#13143: 24.2; mpc.el: mpc-songs-refresh: Args out of range

bug#13143: 24.2; mpc.el: mpc-songs-refresh: Args out of range

[David Cadé]

This bug report will be sent to the Bug-GNU-Emacs mailing list
and the GNU bug tracker at debbugs.gnu.org.  Please check that
the From: line contains a valid email address.  After a delay of up
to one day, you should receive an acknowledgement at that address.

Please write in English if possible, as the Emacs maintainers
usually do not have translators for other languages.

Please describe exactly what actions triggered the bug, and
the precise symptoms of the bug.  If you can, give a recipe
starting from `emacs -Q':

Dear Emacs developers,

I first launched the mpc interface by typing M-x mpc. I did not touch
any configuration parameter for this mode.
I then chose an album in the Albums | Playlists window by typing RET on
it. This triggered the following error:

mpc-songs-refresh: Args out of range: "..." 0, -21

for an album name that contains japanese characters and is 45 characters
long. The "..." in the error above contains exactly the name of the
album. The down right window only displays the title of the first song of
the album, and nothing else.

I have redacted out the name of the album because this error occurs
with a maybe different and always negative second number for every
sufficiently long album name containing japanese characters. 

Another maybe related bug: when there are japanese characters in the
album name and it is too long to be displayed without ellipsis (…), the
longer the less characters are present before the ellipsis.

If Emacs crashed, and you have the Emacs process in the gdb debugger,
please include the output from the following gdb commands:
    `bt full' and `xbacktrace'.
For information about debugging Emacs, please read the file
/usr/share/emacs/24.2/etc/DEBUG.


In GNU Emacs 24.2.1 (x86_64-pc-linux-gnu, GTK+ Version 2.24.10)
 of 2012-09-09 on trouble, modified by Debian
Configured using:
 `configure '--build' 'x86_64-linux-gnu' '--build' 'x86_64-linux-gnu'
 '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib'
 '--localstatedir=/var/lib' '--infodir=/usr/share/info'
 '--mandir=/usr/share/man' '--with-pop=yes'
 '--enable-locallisppath=/etc/emacs24:/etc/emacs:/usr/local/share/emacs/24.2/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.2/site-lisp:/usr/share/emacs/site-lisp'
 '--with-crt-dir=/usr/lib/x86_64-linux-gnu' '--with-x=yes'
 '--with-x-toolkit=gtk' '--with-toolkit-scroll-bars'
 'build_alias=x86_64-linux-gnu' 'CFLAGS=-g -O2 -fstack-protector
 --param=ssp-buffer-size=4 -Wformat -Werror=format-security -Wall'
 'CPPFLAGS=-D_FORTIFY_SOURCE=2''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: nil
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Fundamental

Minor modes in effect:
  show-paren-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
ESC [ > 0 ; 1 1 5 ; 0 c ESC x e m DEL DEL r e p o TAB 
r TAB RET ESC O A ESC O A m p c : SPC m p c - r e l 
o a n DEL DEL DEL DEL DEL DEL TAB TAB DEL DEL s o n 
g s - r e l o a DEL C-g C-x b C-g C-x C-f e m TAB RET 
C-x b C-g ESC x e m a TAB r e p TAB DEL DEL DEL DEL 
DEL DEL DEL DEL DEL r e p TAB o TAB r TAB RET

Recent messages:
Startup with window[1]
Loading /etc/emacs/site-start.d/50windows-el.el (source)...done
Loading /etc/emacs/site-start.d/51ghc-mod.el (source)...done
Loading /etc/emacs/site-start.d/51tuareg-mode.el (source)...done
Loading iso-transl...done
For information about GNU Emacs and the GNU system, type C-h C-a.
Making completion list...
goto-history-element: Beginning of history; no preceding item [2 times]
Quit [3 times]
Making completion list... [2 times]

Load-path shadows:

bug#13143: 24.2; mpc.el: mpc-songs-refresh: Args out of range

[Stefan Monnier]

> I first launched the mpc interface by typing M-x mpc. I did not touch
> any configuration parameter for this mode.
> I then chose an album in the Albums | Playlists window by typing RET on
> it.  This triggered the following error:
> mpc-songs-refresh: Args out of range: "..." 0, -21

I can't reproduce this here; I don't have any albums with japanese
characters, but I when I tried with albums with non-ASCII chars, it
worked fine.
Could you select "Options => Enter debugger on error" and reproduce this bug?
Hopefully this will give us a backtrace that gives us some clue of
what's going on.

> Another maybe related bug: when there are japanese characters in the
> album name and it is too long to be displayed without ellipsis (…), the
> longer the less characters are present before the ellipsis.

You mean that if the real title is longer, then it gets
truncated earlier?  As in "toto" would be truncated to "tot…" but
"wonderful" would be truncated to just "w…" rather than "won…"?

Hmm... that might be a clue, indeed, tho it might be
completely unrelated.


        Stefan

bug#13143: 24.2; mpc.el: mpc-songs-refresh: Args out of range

[Thierry Volpiatto]

Hi Stefan,

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> You mean that if the real title is longer, then it gets
> truncated earlier?  As in "toto" would be truncated to "tot…" but
> "wonderful" would be truncated to just "w…" rather than "won…"?
>
> Hmm... that might be a clue, indeed, tho it might be
> completely unrelated.
I had a similar bug here in helm with japonese chars, see:
https://github.com/emacs-helm/helm/issues/170

HTH, maybe not related though.

-- 
  Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 

bug#13143: 24.2; mpc.el: mpc-songs-refresh: Args out of range

[David Cadé]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2569 bytes --]

Hello,

On Tue, 11 Dec 2012, Stefan Monnier wrote:

>> I first launched the mpc interface by typing M-x mpc. I did not touch
>> any configuration parameter for this mode.
>> I then chose an album in the Albums | Playlists window by typing RET on
>> it.  This triggered the following error:
>> mpc-songs-refresh: Args out of range: "..." 0, -21
>
> I can't reproduce this here; I don't have any albums with japanese
> characters, but I when I tried with albums with non-ASCII chars, it
> worked fine.
>
> Could you select "Options => Enter debugger on error" and reproduce this bug?
> Hopefully this will give us a backtrace that gives us some clue of
> what's going on.

Here you go:

Debugger entered--Lisp error: (args-out-of-range "魔法少女リリカルなのは　サウンドステージ01 第2.5話「ドキ！水着でプールで大ピンチなの」" 0 -21)
   mpc-format("%2{Disc--}%3{Track} %-5{Time} %25{Title} %20{Album} %20{Artist} %10{Date}" ((file . "田村ゆかり・他 - 魔法少女リリカルなのは　サウンドステージ01 第2.5話「ドキ！水着でプールで大ピンチなの」/01 - 「なのは魔法の練習中」.flac") (Last-Modified . "2010-10-26T01:22:26Z") (Time . "295") (Title . "「なのは魔法の練習中」") (Artist . "田村ゆかり・他") (Album . "魔法少女リリカルなのは　サウンドステージ01 第2.5話「ドキ！水着でプールで大ピンチなの」") (Genre . "Anime") (Track . "1") (Date . "2004-01-01T00:00:00")))
   mpc-songs-refresh()
   mpc-selection-refresh()
   mpc-select(13)
   call-interactively(mpc-select nil nil)

>> Another maybe related bug: when there are japanese characters in the
>> album name and it is too long to be displayed without ellipsis (…), the
>> longer the less characters are present before the ellipsis.
>
> You mean that if the real title is longer, then it gets
> truncated earlier?  As in "toto" would be truncated to "tot…" but
> "wonderful" would be truncated to just "w…" rather than "won…"?

Exactly.

> Hmm... that might be a clue, indeed, tho it might be
> completely unrelated.

My take on the bug:
I looked at the code of this function, and from what I can gather, the bug 
seems to be in:
(substring text 0 (- size postwidth textwidth 1))
postwidth and textwidth are widths obtained with string-width, that is to 
say in number of columns, and may not have any relation with the number of 
characters present in the string text.
This seems wrong to me. It seems to me that truncate-string-to-width seems 
to be the function one would want to use in this case.

Greetings,

-- 
David

bug#13143: 24.2; mpc.el: mpc-songs-refresh: Args out of range

[David Cadé]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 742 bytes --]

On Wed, 12 Dec 2012, David Cadé wrote:

> My take on the bug:
> I looked at the code of this function, and from what I can gather, the bug 
> seems to be in:
> (substring text 0 (- size postwidth textwidth 1))
> postwidth and textwidth are widths obtained with string-width, that is to say 
> in number of columns, and may not have any relation with the number of 
> characters present in the string text.
> This seems wrong to me. It seems to me that truncate-string-to-width seems to 
> be the function one would want to use in this case.

This patch in attachment makes things work for me.

Stefan, I don't know if you are registered to this bug number, and I 
apologize if you received multiple copies of this mail.

Greetings,

-- 
David

[-- Attachment #2: Type: TEXT/PLAIN, Size: 731 bytes --]

=== modified file 'lisp/mpc.el'
--- lisp/mpc.el	2012-08-15 16:29:11 +0000
+++ lisp/mpc.el	2012-12-12 13:53:32 +0000
@@ -1036,9 +1036,7 @@
                             (> (+ postwidth textwidth) size))
                        ;; This doesn't even obey double-width chars :-(
                        (propertize
-                        (if (zerop (- size postwidth 1))
-                            (substring text 0 1)
-                          (concat (substring text 0 (- size postwidth textwidth 1)) "…"))
+			(truncate-string-to-width text size nil nil "…")
                         'help-echo text)
                      text)))
               (when (memq tag '(Artist Album Composer)) ;FIXME: wrong list.

bug#13143: 24.2; mpc.el: mpc-songs-refresh: Args out of range

[Stefan Monnier]

Version: 24.4

> I looked at the code of this function, and from what I can gather, the bug
> seems to be in:
> (substring text 0 (- size postwidth textwidth 1))

Yes, that's also what I bumped into.  Great minds debug alike!

The patch looks good, installed in `trunk', thank you very much.


        Stefan


PS: I guess I didn't know about truncate-string-to-width when I wrote
this code.  But looking at truncate-string-to-width, I hope it won't
impact performance too much.

bug#13143: 24.2; mpc.el: mpc-songs-refresh: Args out of range

[Bastien]

Hi Stefan,

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> PS: I guess I didn't know about truncate-string-to-width when I wrote
> this code.  But looking at truncate-string-to-width, I hope it won't
> impact performance too much.

Argh!  truncate-string-to-width does what I suggested as string-tail:
http://lists.gnu.org/archive/html/emacs-devel/2012-07/msg00357.html

Maybe an alias like string-truncate-to-width (or just string-truncate)
could help discoverability?

-- 
 Bastien

bug#13143: 24.2; mpc.el: mpc-songs-refresh: Args out of range

[David Cadé]

On Wed, 12 Dec 2012, Bastien wrote:

> Argh!  truncate-string-to-width does what I suggested as string-tail:
> http://lists.gnu.org/archive/html/emacs-devel/2012-07/msg00357.html

<pedantic mode>
Your string-tail implementation limits the string to n characters, but 
truncate-string-to-width limits it to n columns, which is not the same.
(There are characters that take 0 (combining characters) 1 or 2 (full 
width CJK characters) columns)
</pedantic mode>

Greetings,

-- 
David

bug#13143: 24.2; mpc.el: mpc-songs-refresh: Args out of range

[Bastien]

Hi David,

David Cadé <codename68@gmail.com> writes:

> On Wed, 12 Dec 2012, Bastien wrote:
>
>> Argh!  truncate-string-to-width does what I suggested as string-tail:
>> http://lists.gnu.org/archive/html/emacs-devel/2012-07/msg00357.html
>
> <pedantic mode>
> Your string-tail implementation limits the string to n characters, but
> truncate-string-to-width limits it to n columns, which is not the same.
> (There are characters that take 0 (combining characters) 1 or 2 (full width
> CJK characters) columns)
> </pedantic mode>

Agreed.  I just wish I'd stumbled on `truncate-string-to-width' before!

-- 
 Bastien