Re: [emms-help] Emms lyrics bug

[Yoni Rabkin]

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"