Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

which-key-key-based-description-replacement-alist not working? #62

Closed
kaushalmodi opened this issue Sep 18, 2015 · 13 comments
Closed

which-key-key-based-description-replacement-alist not working? #62

kaushalmodi opened this issue Sep 18, 2015 · 13 comments

Comments

@kaushalmodi
Copy link

Hi,

I have the which-key-key-based-description-replacement-alist variable customized as below:

   (setq which-key-key-based-description-replacement-alist
          '(("C-x 8"   . "unicode")
            ("C-x a"   . "abbrev/expand")
            ("C-x r"   . "rect/reg")
            ("C-x w"   . "hi-lock-map")
            ("C-c /"   . "engine-mode-map")
            ("C-c C-v" . "org-babel")
            ("C-x 8 0" . "ZWS")))

But it is not working. I expected to see "8 → unicode" after pressing "C-x", but instead I see "8 → +prefix".

I have seen this feature to work fine; but it probably broke in the updates that happened in last 2-3 weeks.

I can see the same problem on both emacs 24.5 and the latest emacs build from git.

@justbur
Copy link
Owner

justbur commented Sep 18, 2015

Sorry! Yes, I broke that way of doing it. I switched to a better way of storing the keys that doesn't rely on strings. This should work for you

(which-key-add-key-based-replacements
  "C-x 8" "unicode"
  "C-x a" "abbrev/expand"
...
)

@justbur
Copy link
Owner

justbur commented Sep 18, 2015

I just realized that I didn't update the README. Doing that now..

@kaushalmodi
Copy link
Author

Cool! I will try that. While you are doing that, you can also mark that var as deprecated.

@justbur
Copy link
Owner

justbur commented Sep 18, 2015

Well... It's not deprecated because I'm still using it. Maybe I screwed up, but my intention was for people to use the helper functions, which are used the same way. At least the readme is correct now. Sorry for the confusion.

@kaushalmodi
Copy link
Author

No problem. Your suggested method brings back the functionality. Thanks for the quick reply!

@kaushalmodi
Copy link
Author

If you wish, you can make that a defvar instead of a defcustom and rename it to which-key--key-based-description-replacement-alist (Note the which-key-- prefix). That will give a clear indication that users are not supposed to tinker with it.

@kaushalmodi kaushalmodi reopened this Sep 18, 2015
@justbur
Copy link
Owner

justbur commented Sep 18, 2015

That's a good idea. I'll do that soon. Thanks

@justbur
Copy link
Owner

justbur commented Sep 18, 2015

@kaushalmodi Do you think I should use a message or a warning for the old variable?

@kaushalmodi
Copy link
Author

I believe a warning would be justified as it's a signification change in the structure of that alist.

I had a related suggestion; why not merge these two into a single function and set the major mode specific options ".dir-locals.el style":

Before

(which-key-add-key-based-replacements
  "C-x C-f" "find files")
(which-key-add-major-mode-key-based-replacements 'org-mode
  "C-c C-c" "Org C-c C-c"
  "C-c C-a" "Org Attach")

After

(which-key-add-key-based-replacements
  '((nil . (("C-x C-f" . "find files")))
    (org-mode . (("C-c C-c" . "Org C-c C-c")
                 ("C-c C-a" . "Org Attach")))))

This might need a quite some code reworking. So it can be understood if you choose not to take this approach.

@justbur
Copy link
Owner

justbur commented Sep 18, 2015

I think what I'll do is switch to defvar as you suggested but not change the name. I can put some code in there to detect if the old style of key string is being used and warn then, but also do the conversion automatically.

Thanks, I'll think about your suggestion for the function change. I think I find the current way more readable at a glance.

@justbur
Copy link
Owner

justbur commented Sep 18, 2015

I pushed the changes. I opted not to change the variable names, but just to add code that fixes the old format if necessary for backwards compatibility. It will print a message now if it detects the old format.

Thanks for the report. Your old code should work now too, actually, but the new way will be preferred going forward.

@justbur
Copy link
Owner

justbur commented Sep 21, 2015

@kaushalmodi Are you happy with the solution?

@kaushalmodi
Copy link
Author

Yes, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants