emms-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Emms-patches] support for DISCNUMBER metadata


From: Yoni Rabkin
Subject: Re: [Emms-patches] support for DISCNUMBER metadata
Date: Thu, 20 Sep 2012 08:10:47 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux)

Christophe Rhodes <address@hidden> writes:

> Yoni Rabkin <address@hidden> writes:
>
>>> Sorry for the delay, I've been away from the project for a while. I'll
>>> have a look at this patch and submit it if it works.
>>
>> I don't have FLAC files, much less multi-disc FLAC files to test the
>> patch, but is seems sane and it doesn't seem to break any existing
>> functionality. I went ahead and submitted it.
>>
>> Thank you for your work.
>>
>> Lucas, do we need FSF papers for Mr. Rhodes?
>
> I have not sent FSF papers, so if that change is sufficiently exciting,
> I guess I should send some.
>
> In the meantime, there is another change that is probably not
> substantial enough to require papers but needs doing: the logic in
> case-fold-string= is wrong, because `compare-strings' returns t or a
> number, and so always returns a boolean true.  (This leads to all sorts
> of weird behaviour in the browser as soon as there are hash
> collisions).  I'm appending an inline diff below.

Or simply (eq t (compare-strings ...))

I went ahead and made the change. Thanks for catching this.

> (I don't know how to propagate changes further, but this mistake, if I'm
> right, is also present in the elisp reference manual, or at least the
> version at
> <http://www.gnu.org/software/emacs/manual/html_node/elisp/Defining-Hash.html>).
> Any advice on getting that changed would be welcome.

In the Emacs source code this is done correctly, but the manual is
wrong. I guess that the right way to fix this is to file a bug against
it.

> Thanks,
>
> Christophe
>
> diff --git a/lisp/emms-browser.el b/lisp/emms-browser.el
> index c52f326..f5f54bd 100644
> --- a/lisp/emms-browser.el
> +++ b/lisp/emms-browser.el
> @@ -674,7 +674,8 @@ compilations, etc."
>       (sort-lines nil (point-min) (point-max)))))
>  
>  (defun case-fold-string= (a b)
> -  (compare-strings a nil nil b nil nil t))
> +  (if (eql (compare-strings a nil nil b nil nil t) t)
> +      t nil))
>  
>  (defun case-fold-string-hash (a)
>    (sxhash (upcase a)))
>
>
> _______________________________________________
> Emms-patches mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/emms-patches
>

-- 
   "Cut your own wood and it will warm you twice"



reply via email to

[Prev in Thread] Current Thread [Next in Thread]