[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [emms-help] Emms lyrics bug
From: |
Yoni Rabkin |
Subject: |
Re: [emms-help] Emms lyrics bug |
Date: |
Wed, 21 Dec 2016 07:18:16 -0500 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) |
Yoni Rabkin <address@hidden> writes:
> Rasmus <address@hidden> writes:
>
>> Yoni Rabkin <address@hidden> writes:
>>
>>> Hello,
>>>
>>> A handful of compilation bugs are left. Among them:
>>>
>>> In emms-lyrics-visit-lyric:
>>> emms-lyrics.el:240:53:Warning: reference to free variable ‘filename’
>>> emms-lyrics.el:248:47:Warning: reference to free variable
>>> ‘eww-after-render-hook’
>>>
>>> These seem to trace back to Rasmus' commit
>>> 6bc53c76eadaee1ba37d6301c28ac987d77c0124 from 2015.
>>>
>>> Rasmus, can you please fix that issue by either re-binding filename to
>>> the right value in that `let' or doing away with it altogether?
>>
>> I don't understand why it's unhappy about filename.
>
> Because filename is unbound. It used to be bound in the enclosing `let',
> but you removed that.
To be more specific, here is the function. Note that filename isn't
bound, but referenced.
(defun emms-lyrics-visit-lyric ()
"Visit playing track's lyric file.
If we can't find it from local disk, then search it from internet."
(interactive)
(let* ((track (emms-playlist-current-selected-track))
(name (emms-track-get track 'name))
(lrc (funcall emms-lyrics-find-lyric-function
(emms-replace-regexp-in-string
(concat "\\." (file-name-extension name) "\\'")
".lrc"
(file-name-nondirectory name)))))
(if (and lrc (file-exists-p lrc) (not (string= lrc "")))
(find-file lrc)
(message "Lyric file does not exist on file-system. Searching online...")
(let* ((title (or (emms-track-get track 'info-title)
(file-name-sans-extension
(file-name-nondirectory name))))
(artist (when (emms-track-get track 'info-title)
(emms-track-get track 'info-artist)))
(url
(cond ((string-match "\\cc" title) ; Chinese lyrics.
;; Since tag info might be encoded using various coding
;; systems, we'd better fall back on filename.
(format emms-lyrics-chinese-url
(emms-url-quote-plus
(encode-coding-string filename 'gb2312))))
(t ; English lyrics.
(format emms-lyrics-latin-url
(if artist (concat (emms-url-quote-underscore
artist) ":") "")
(emms-url-quote-underscore title))))))
(if (fboundp 'eww)
(let ((readable-hook (when (and (fboundp 'eww-readable)
(not (memq 'eww-readable
eww-after-render-hook)))
(add-hook 'eww-after-render-hook
'eww-readable))))
(eww url)
(when readable-hook
(remove-hook 'eww-after-render-hook 'eww-readable)))
(browse-url url))
(message "Lyric file does not exist on file-system. Searching
online...")))))
...and there is how filename was removed from in commit
6bc53c76eadaee1ba37d6301c28ac987d77c0124:
"""
- (let ((title (emms-track-get track 'info-title))
- (filename (file-name-sans-extension
- (file-name-nondirectory name)))
- (url ""))
"""
>> In any case, I cannot really test it at the moment as EMMS is not
>> compatible with the master branch of Emacs. It seems EMMS uses
>> default-major-mode, which is not part of Emacs anymore:
>
> I just changed that yesterday. It now uses `major-mode':
> http://git.savannah.gnu.org/cgit/emms.git/commit/?id=ea9b8906c773b722d1c06c62bf5883b5a6de5342
>
>> *** All the default-FOO variables that hold the default value of the
>> FOO variable. Use 'default-value' and 'setq-default' to access and
>> change FOO, respectively. The exhaustive list of removed variables is:
>> 'default-mode-line-format', 'default-header-line-format',
>> 'default-line-spacing', 'default-abbrev-mode', 'default-ctl-arrow',
>> 'default-truncate-lines', 'default-left-margin', 'default-tab-width',
>> 'default-case-fold-search', 'default-left-margin-width',
>> 'default-right-margin-width', 'default-left-fringe-width',
>> 'default-right-fringe-width', 'default-fringes-outside-margins',
>> 'default-scroll-bar-width', 'default-vertical-scroll-bar',
>> 'default-indicate-empty-lines', 'default-indicate-buffer-boundaries',
>> 'default-fringe-indicator-alist', 'default-fringe-cursor-alist',
>> 'default-scroll-up-aggressively', 'default-scroll-down-aggressively',
>> 'default-fill-column', 'default-cursor-type',
>> 'default-cursor-in-non-selected-windows',
>> 'default-buffer-file-coding-system', 'default-major-mode', and
>> 'default-enable-multibyte-characters'.
>>
>> Eww is part of newer versions of Emacs. Perhaps we can make the hook
>> conditional on (featurep ’eww). I don’t know if this would silence the
>> byte-compiler (I can’t test cf. above.)
>>
>> Perhaps it’s best to revert the patch for now. It was an attempt to make
>> emms-lyrics work to some extend (as it was completely broken), but it
>> might not be worth it.
>>
>> Thanks,
>> Rasmus
--
"Cut your own wood and it will warm you twice"