Fix customization and documentation for spotify-keymap-prefix

[andersjohansson]

• Add a customize set function for correctly redefining spotify-mode-map when spotify-keymap-prefix is set
• Update documentation to reflect this change

In particular, I found that the suggestion to do (define-key spotify-mode-map (kbd "C-c .") 'spotify-command-map) in the readme conflicted with the spotify-keymap-prefix variable.

[andersjohansson]

@jkdufair may also want to have a look. Using :set for custom variables is often a bit tricky but it seems to work correctly in my testing.

[danielfm]

Hi @andersjohansson, thanks for your contribution!

It's still not clear to me why this PR is needed. Is there a bug it fixes? Can you describe a step-by-step procedure on how to reproduce it?

[jkdufair]

This seems ok to me. I would still provide a default (and leave the docs with that default) so it works “out of the box”. But I like the ability to use customize. We could probably stand to make more vars available via customize

[andersjohansson]

Sorry for not explaining the reasoning well enough. The point was that looking at the new customizability options for the keymap introduced by @jkdufair, it seemed to me that there were two different recommended ways of setting your own prefix. First, the custom variable. Second, in the readme, the suggestion to do (define-key spotify-mode-map (kbd "C-c .") 'spotify-command-map).

I thought this looked inconsistent and that it would be easier if there was a single recommended way of setting the keymap prefix (even though the define-key route above still works). For this, I expanded the functionality of the custom variable to correctly update the keymap when set (via customize, actually how to do the same thing if the variable is set outside customize should probably be documented in the docstring).

As I understand @jkdufair, maybe the default for spotify-keymap-prefix should be C-c ., so that there’s some binding "out of the box". However, this wasn’t the case before my changes (unless users did the define-key binding in their own configs).

[danielfm]

Thanks for the explanation.

I tried the code from this PR and I get the following error when I try to (require 'spotify):

Warning (initialization): An error occurred while loading ‘/home/danielfm/.emacs.d/init.el’:

Symbol's function definition is void: spotify-set-mode-map

To ensure normal operation, you should investigate and remove the
cause of the error in your initialization file.  Start Emacs with
the ‘--debug-init’ option to view a complete error backtrace.

[danielfm]

Just found that it worked after moving the spotify-sete-mode-map function before the defcustom referencing it.

However, I was not able to set the prefix via customize, it didn't seem to work even after I restarted emacs. I only managed to set the prefix by running (setq spotify-keymap-prefix "C-c .") before (require 'spotify), as documented in the README.

[andersjohansson]

Ah, I should have known. I seem to always bite myself in some way when using :set functions. I will try to sort it out if you still think it’s a worthwhile approach. I mean, we could instead just remove the custom variable, ask the user to use define-key as earlier and be done with it. But if the custom variable exists, I think that setting it should update the mode-map (and not only build it correctly when spotify-remote.el is loaded).