use-package integration

[blaenk]

Hey noctuid! You might remember me as a user of your link-hint.el package. I just now realized you have other great packages such as this one.

It would be pretty cool if you created use-package integration, so that we could use a keyword like the existing :bind, but I guess named differently like :general or something preferably not too long.

I'd definitely use it because it makes things more organized and clean, I've found. The one problem I had with the :bind keyword in use-package, which uses the bind-key package, was that it didn't seem to wait for the keymaps to exist, so I'd sometimes have to break from the norm of using :bind to actually explicitly creating the bind in the :config section or something. So even in that specific use case general would be of great help! It's very nice to be able to have the binds grouped up in one place for each use-package.

With respect to what its arguments should be, notice how :bind has a variety of different features, from the readme:

(use-package color-moccur
  :commands (isearch-moccur isearch-all)
  :bind (
         ("M-s O" . moccur)

         :map isearch-mode-map
         ("M-o" . isearch-moccur)
         ("M-O" . isearch-moccur-all)
  )
  :init
  (setq isearch-lazy-highlight t)
  :config
  (use-package moccur-edit))

There you can see how it can take a :map argument to specify the keymap on which to bind, so the subsequent list is bound to that map.

It would be cool if we could come up with a similar AST-ish thing for the various features that general supports.

One additional and crucial request would be the ability to interject an existing general wrapper (i.e. created with general-create-definer), so that subsequent forms are based on it. So for example, something like:

(general-create-definer my-nmap :keymaps 'evil-normal-state-map)

:general (
  ;; as if general-define-key called with defaults
  ("C-c a" 'some-command
   "C-c b" 'another-command)

  ;; general-define-key called with :prefix "C-x"
  :prefix "C-x"
  ("j" 'jump
   "p" 'paste)

  ;; using the my-nmap wrapper defined above, so those defaults are used
  ;; preferably this could be followed by "overrides" like I think is already possible
  ;; with normal wrappers, e.g. if this were followed by :keymaps 'evil-insert-state-map
  ;; to override the default defined in the wrapper. in this case it defeats the purpose
  ;; of using a wrapper since that was the only default it specified, but you get my point.
  :defaults my-nmap
  ("o" 'thing
   "a" 'other)
)

Separated out with whitespace to better convey what I mean.

So I guess each sub-list can be preceded by arguments that would go to general-define-key, and the sublist that follows it are affected by those defaults. After the sublist, that can be repeated on a blank slate. So for example when it gets to :defaults my-nmap, that's from a blank slate, the earlier :prefix "C-x" no longer applies.

This would be absolutely amazing. Perhaps you can think of a better name than :defaults, but my point is it would be great to have something like that. In the absolute worst case I guess you can just create a separate keyword for it, e.g. :general-with-defaults my-nmap, but I would highly prefer and encourage a uniform :general keyword, just like how use-package lately seems to be converging upon a single :bind keyword based on recent commits I've seen.

Also like :bind, the presence of a :general form should imply :defer t or whatever.

[noctuid]

What do you see as the benefits of this over just having general-define-key functions in the :init section? I could add a new wrapper with syntax that would allow changing the prefix or keymap for later keybindings like you suggest, but this seems more messy to me than using multiple specialized general- functions. The suggested syntax would also be more messy to implement since I'd have to manually parse the keywords. If added, I think I'd prefer to just keep the syntax the same as general-define-key. It would look something more like this:

:general
(("SPC" #'command)
 ("SPC" #'command
  :keymaps 'org-mode-map))

That way each sublist would just correspond to the arglist for a single general-define-key. In addition to being much easier to implement, I think this would be less confusing. I could maybe add a keyword to general-define-key to choose a set of defaults as an alternative to using the general-create-definer wrappers (or maybe combine the two?). I have to think about what would be the best way to do this.

should imply :defer t or whatever

So it would also add autoloads for the bound commands? I don't find this feature useful, but I could maybe reuse use-package's code to do that.

[blaenk]

The reason I like writing out my binds separate from :init or :config is because it feels like a nicer separation. I can see at a glance what binds I created for a particular package, not for looking up during use (for that I have helm-descbinds) but for maintaining. It keeps things much more organized. Add to that the fact that general supports waiting until the keymap becomes available and it seems like the perfect combination.

I don't have a particular preference for how the syntax looks, I was just basing it off of how :bind works in use-package, which uses bind-key. I'd simply be happy if the functionality I want is possible, both anything that's possible with general-define-key as well as leveraging existing defaults/wrappers.

So you mean something like this would be simpler to implement? If so, sounds fine to me:

(general-create-definer my-nmap :keymaps 'evil-normal-state-map)

:general (
  ("C-c a" 'some-command
   "C-c b" 'another-command)

  ("j" 'jump
   "p" 'paste
   :prefix "C-x")

  ("o" 'thing
   "a" 'other
   :defaults my-nmap)
)

Did you purposefully put the arguments at the end of the keys? Or could it be like this too:

  (:prefix "C-x"
   "j" 'jump
   "p" 'paste)

It's no difference to me, though I do tend to expect that stuff to be at the beginning since it makes clear why it's a different sublist.

I guess it'd be cool if we could omit the outer parentheses, like how :config and :init don't require surrounding delimiters, but that's a minor thing that's not a big deal.

I could maybe add a keyword to general-define-key to choose a set of defaults as an alternative to using the general-create-definer wrappers (or maybe combine the two?).

If you make it an argument, indeed I'd like to be able to use the name of an existing wrapper but I'm not sure if that'd be possible. I'm not an expert in elisp but IIRC you can give a variable a function value as well as a regular value, or something. So if you give it a regular value, that would contain the defaults which general-define-key would apply? And yeah that way general-define-key could take a :defaults some-wrapper argument for example.

The reason I'd prefer to be able to use an existing wrapper's defaults is so that I could still use the wrapper if I want to. Otherwise it seems like I'd have to specify the defaults twice? Both for use as a function and for use in the use-package keyword?

Or do you mean something like having a variable with a list of defaults like:

(setq my-defaults '(:prefix "C-c" :states '(emacs insert))

Then when creating the wrapper you do:

(general-create-definer my-map :defaults my-defaults)

Then in use-package :general just name the defaults variable:

  ("o" 'thing
   "a" 'other
   :defaults my-defaults)

I guess that would work too but if it's possible to give a value to the wrapper then might as well use that, but I'm fine either way.

So it would also add autoloads for the bound commands? I don't find this feature useful, but I could maybe reuse use-package's code to do that.

Oh no if it's too much trouble don't worry about it. Perhaps I misunderstand use-package, but I just meant how when you :bind something, it implies that the package shouldn't be loaded right away but instead it'll be loaded on the autoload when the bind is pressed. This way it wouldn't be necessary to explicitly :defer t. I wouldn't worry about actually adding autoloads, I'd leave that to use-package or the package itself or the user. So simply :general would imply :defer t. But again if it's too much trouble then I'd just write out the :defer t myself, no problem.

[noctuid]

So you mean something like this would be simpler to implement?

Yes since it wouldn't require any changes from general-define-key. I don't care too much about whether something is hard to implement, but in this case I like this syntax more beause it doesn't require any new rules or explanation.

Or could it be like this too

Yes, the keyword location wouldn't matter.

I guess it'd be cool if we could omit the outer parentheses

I assume that with how use-package handles these keywords, it might also be easily possible to do this with a :general keyword. I'll look into it.

The reason I'd prefer to be able to use an existing wrapper's defaults is so that I could still use the wrapper if I want to.

Yeah I think it would be best to not have to create the defaults twice. I like your first suggestion and will do it that way (but the second method could be possible with :defaults added). I won't make any changes to how general-create-definer works currently, but it will additionally define a variable with the same name containing a list of the default arguments for use with a :defaults keyword.

As for implying :defer t, I imagine it would be easy to do. I brought up the autoloads because the use-package keywords that imply deferral make autoloads. For the case of :bind, it's almost always redudant. :commands exists if really needed, so for now I won't do anything with autoloads. Still, it's potentially useful in a few circumstances, so I may add an :autoload keyword to general-define-key in the future that would take the package name to create autoloads with.

[blaenk]

Sounds awesome! Happy to see we're in agreement.

I was thinking about this some more and I remembered one minor thing. For example, I'm used to the bind-key function which is comparatively shorter than general-define-key, so I created a wrapper with the definer simply called bind, with no defaults specified. Then I wanted to create a wrapper for my leader-ish binds which specify a :prefix for example, which I also wanted to be short, so I called it bind*. I'm still not a complete fan of these names I created but they have the advantage of being short for now.

That said, with this stuff we're talking about, I wouldn't need to use the bind wrapper defaults because that doesn't contain any defaults, it's literally just (general-create-definer bind), actually now that I think about it, perhaps I should just make it a defalias? Although I'm not sure if defalias would carry over the non-function value once you implement this, I think it only sets the function value, so I guess I'll keep it how I have it. But anyways, I will want to refer to the bind* defaults, so for example I'd have:

:general
  ;; global/no-defaults bindings
  ("C-c a" 'some-command
   "C-c b" 'another-command)

  ;; these would use the defaults that bind* was defined with, e.g. the :prefix I set
  ("o" 'thing
   "a" 'other
   :defaults bind*)

I guess it reads kind of weird, since the function name is more like a verb, but that's definitely my fault. For example something like general-nvmap reads more correctly like :defaults general-nvmap or even :defaults nvmap, so I guess I should rename my bind* wrapper to something else, like my-map or something. You have any suggestions? I guess I'm just thinking aloud here and curious what your opinions are if you have any.

I guess it also depends on the name you end up choosing. For example if instead of :defaults we go with something like :with or :via then perhaps that makes a bit more sense:

:general
  ;; global/no-defaults bindings
  ("C-c a" 'some-command
   "C-c b" 'another-command)

  ;; these would use the defaults that bind* was defined with, e.g. the :prefix I set
  ("o" 'thing
   "a" 'other
   :with bind*)

[blaenk]

Yes since it wouldn't require any changes from general-define-key. I don't care too much about whether something is hard to implement, but in this case I like this syntax more beause it doesn't require any new rules or explanation.

Yeah I agree. No point in changing it since it would require learning two different ways to use it. This is something I myself have encountered with bind-key's :bind in use-package. I always have to look up examples to remember what the DSL is like.

[noctuid]

For your bind, I'd just use defalias. I'd probably rename bind* to something like my-prefix-map depending on the prefix. I think I'd rather use :defaults since it seems more immediately clear what it does, but I'll think about using something like :with.

I went ahead and added a :general keyword as discussed (it takes one or more lists and they do not need to be in a containing list) but only tested it briefly. Looking at how :bind does autoloads, I decided to just copy it since all it does is add the commands to :commands, so :general behaves the same as :bind with regards to deferral and autoloads. Let me know if you encounter any problems. I'll add a keyword for defaults next time I get the chance.

[blaenk]

That's great! I'll give it a shot, thank you very much!

I'm not a huge fan of renaming to my-prefix-map, it feels too long and it's something I may use often. I guess if you could have something like :with which is just an alias of :defaults, but I can understand if you'd consider that cruft and rather not add it. In that case I'll keep thinking about a name to use. Like if I called it my-map it then changes to not feeling very appropriate for using as a function, i.e. (my-map "k" 'thing), although I guess I can read it as simply adding those bindings to that "map", though it's not really a map AFAIK, but that's starting to just split hairs. Yeah I'll see what I can come up with.

[noctuid]

Like if I called it my-map it then changes to not feeling very appropriate for using as a function

Well map is pretty commonly used as a command for defining keys in various programs, so it sounds fine to me.

One problem with creating a variable is that the vim definers can act differently depending on general-vim-definer-default (they don't have a single set of defaults) which would require a special workaround. The other way I was thinking of would be to just give an alternate function name to pass the arguments to. :via would make sense for this. In this case, :via would only really make sense for use with use-package. Alternately, I could not add any keywords and allow optionally specifying an alternate function to pass the keywords to in the :general section:

:general
general-nmap
("key" #'func)

[blaenk]

One problem with creating a variable is that the vim definers can act differently depending on general-vim-definer-default (they don't have a single set of defaults) which would require a special workaround.

Yeah I'm not sure I follow the details on that, I don't use it so it's of no consequence to me.

Yeah that's something I was thinking about like with :via and :with.

Your final example also looks reasonable as well. It feels a bit more weird but I think I can get used to it. I guess whichever is easier to implement. I guess it also reads better than having a :via in the parens.

[blaenk]

So I gave it a shot and so far it works pretty amazingly well. I have some observations and questions.

1. Sometimes I need to unbind a key to then bind a prefix to it. I'm not sure of the terminology, but I mean something like this:

    (bind :states 'normal
      ;; unmap these so they could be used as prefix keys
      ;; this is useful for smartparens
      "<" nil
      ">" nil
   
      ;; still able to shift things in normal mode
      "< <" 'evil-shift-left-line
      "> >" 'evil-shift-right-line)

   Basically I free up < and > so that I can add my own bindings I created for smartparens like >i which puts the cursor right before the closing parenthesis of the enclosing sexp and switches to insert mode. They're mostly inspired from vim-sexp-mappings-for-regular-people. They work well. As far as I know I first have to unbind < for example so that I can bind further things on <. The way I know to unbind keys is by setting them to nil.

   What I found is that if I set a key to nil in a :general section I get a "symbol value is void". I have a feeling perhaps everything is treated as if it's already quoted? I was manually quoting things myself just like general-define-key expects by the way.

2. A more general, general.el question: is there a way to avoid having to wrap a call to a general binding in with-eval-after-load 'package-that-contains-commands-bounded-to? So for example I have this:

    (:keymaps 'evil-window-map
      "m k" 'buf-move-up
      "m j" 'buf-move-down
      "m h" 'buf-move-left
      "m l" 'buf-move-right)

   The thing is that 'buf-move-up may or may not exist. It does exist in my setup of course, but with-eval-after-load handles that possibility as far as I know. I also worried that maybe buffer-move might not be loaded yet, though I think it is. So the reason I ask is because I wonder if it means that I couldn't use it in a :general section since I'd have to wrap it in a with-eval-after-load, so I'd have to move it to the :init section? If so it's no big deal, but I'm wondering if I'm overlooking something. For example, I know that general will wait until the keymap being bound to exists, which is awesome.

   I also saw the :predicate argument but that seems like it's for something else? Though I think it may work? Something like :predicate '(featurep 'buffer-move)?

[noctuid]

Yes, you should continue to manually quote the commands. The issue was that nil was being added to :commands; this should be fixed now. Thanks for pointing this out. Also, since you mention vim-sexp-mappings-for-regular-people, I am planning on adding a key theme to replicate those keys in lispyville which may be of interest (but smartparens is not used as the backend).

As for your second question, even with define-key you would not need to wrap that in any eval-after-load. You can bind a key to a command that doesn't exist. As long as there is an autoload for it, it will still work as expected. If you've installed buffer-move.el using package.el, then autoloads for those commands will already exist, but since :general replicates :bind's behavior for adding autoloads, you wouldn't need to worry about that anyway for this example. :predicate is for something else (basically if the predicate is false, it will fall through and attempt to execute something else).

[blaenk]

Oh wow that's great to hear about the mappings for regular people. I think I created a few extra ones that I'll tell you about in case you think they may be useful to replicate as well. I'd definitely be interested in shedding that huge chunk of code I have for that. I only used smartparens because it exposed many many many useful functions for doing things.

Ah okay so no need for eval-after-load. I guess I wondered as well because if the package for some reason weren't installed, eval-after-load would gracefully handle that, because it would never create the bindings since the package would never have loaded. I think that's an unnecessary worry though because I do know it's installed, but I just meant out of an interest for correctness.

You can see what my config looks like so far with this new functionality in this commit, for example my evil configuration. All binds are now grouped together before :init and :config instead of spread out throughout :init and :config. Now I'm just waiting on the functionality for using existing wrappers so I can completely move over to using :general.

[blaenk]

Your fix for unbinding keys confirmed working.

[blaenk]

Oh hey I had a question for you. For wrappers and general-define-key I did something like:

(put 'general-define-key 'lisp-indent-function 'defun)

Because I like to format the binds something like:

(general-define-key :keymaps '(one two) :states '(normal visual)
  "C-z" 'thing
  "C-y" 'other)

In other words like a defun, so that subsequent lines are only indented once.

With the general sublists though that corresponds to something like this:

  :general
  (:states 'emacs
    "C-w" 'evil-window-map)

  (:states 'insert
    "<S-return>" 'comment-indent-new-line

    "C-y" 'blaenk/evil-insert-mode-paste

    "C-u" 'blaenk/kill-line
    "C-l" 'move-end-of-line)

However it keeps wanting to indent to something like:

  :general
  (:states 'emacs
           "C-w" 'evil-window-map)

  (:states 'insert
           "<S-return>" 'comment-indent-new-line

           "C-y" 'blaenk/evil-insert-mode-paste

           "C-u" 'blaenk/kill-line
           "C-l" 'move-end-of-line)

It's somewhat a minor thing because I can indent manually, but I was wondering if you happen to know if it's possible to make these things indent correctly?

[noctuid]

I think I created a few extra ones that I'll tell you about in case you think they may be useful to replicate as well.

Feel free to make an issue on lispyville suggesting these. I'll work on adding those mappings when I get the chance.

eval-after-load would gracefully handle that, because it would never create the bindings since the package would never have loaded

If not wrapped in eval-after-load and the package doesn't exist, it won't immediately cause an error, but the keys will still be bound. They just won't work until you install the package.

Now I'm just waiting on the functionality for using existing wrappers so I can completely move over to using :general.

Okay, I've added it. I decided it would make a lot more sense to just use the regular syntax for wrappers (to use a wrapper, you just put its name first in the list). I've documented this in the last section of the readme. Please test and let me know if you have any problems.

It's somewhat a minor thing because I can indent manually, but I was wondering if you happen to know if it's possible to make these things indent correctly?

Yeah the default indentation function is annoying for lists that start with a keyword. You can fix this by using a different one. I've started adding an emacs-lisp-mode-hook to locally set lisp-indent-function to Fuco1's modified lisp-indent-function. I've also put this in the readme.

[blaenk]

Amazing!! Thank you very much, I'll try all this out!

[blaenk]

Hey I noticed something earlier but I wasn't able to replicate it, but now I can.

Basically I previously would do something like:

(use-package link-hint
  :defer t
  :bind
  ("C-c l o" . link-hint-open-link
   "C-c l c" . link-hint-copy-link)

  :init
  (setq link-hint-avy-style 'post))

I recognize that :defer t and :bind are redundant there, that was why I had asked if you could have :general imply :defer t as well. However, doing the above worked fine, it would still let me use those bindings which would autoload the package. However, if I have :defer t and :general, it seems like if the :general stuff is run after the package loads, as if it were in :config, so if the :defer t is left in, the bindings don't exist and the package is never loaded since the bindings can't be triggered since they don't exist:

(use-package link-hint
  :defer t
  :general
  ("C-c l o" 'link-hint-open-link
   "C-c l c" 'link-hint-copy-link)

  :init
  (setq link-hint-avy-style 'post))

Once I remove :defer t then everything works as expected.

Again, I understand that :defer t is redundant there, but my concern is that it shouldn't actively break things if it's there anyways, if that makes any sense. I think this is especially important because some people (not me) use the use-package-always-defer option which globally implies :defer t for every package unless :demand t is specified, so this can end up silently breaking things unless they specify :demand t as well which would be counter-intuitive and AFAIK would defeat the purpose of autoloading the package.

My way of looking at it was if I had :defer t and :bind as well, clearly the :defer t is unnecessary there but it shouldn't matter because :bind would still work which would cause the package to be loaded when the bind is triggered. I don't actively leave in :defer ts in there but sometimes they're left behind when I'm fiddling around with configuring a particular package, which is why I recognized this.

Is this something that can be resolved? I'm just not going to leave in :defer t from now on so it's not the end of the world for me, but preferably I would've imagine that things shouldn't break if it was left in. I feel like the :general stuff should be executed immediately as if it were in :init no? Especially since general is capable of waiting for the keymap to exist. Then again this is probably what's already being done and the problem is somewhere else.

[noctuid]

I couldn't replicate the issue you described. Keybindings were still added for me even with an explicit :defer t. However I did notice that :bind would work even for a nonexistent package. I've added :general to the use-package-keywords list right after the :bind keywords, so any related issues should be fixed. Let me know if they aren't.

[blaenk]

Am I misinterpreting this? I have this:

(use-package link-hint
  :defer t

  :general
  ("C-c l o" 'link-hint-open-link
   "C-c l c" 'link-hint-copy-link)

  :init
  (setq link-hint-avy-style 'post))

And then I setq use-package-debug t and then I eval-last-sexp on it and the *use-package* buffer that pops up shows this:

(progn
  (progn
    (require 'package)
    (use-package-ensure-elpa 'link-hint))
  (condition-case-unless-debug err
      (setq link-hint-avy-style 'post)
    (error
     (ignore
      (display-warning 'use-package
                       (format "%s %s: %s" "link-hint" ":init"
                               (error-message-string err))
                       :error))))
  (eval-after-load 'link-hint
    '(progn
       (unless
           (fboundp 'link-hint-open-link)
         (autoload #'link-hint-open-link "link-hint" nil t))
       (unless
           (fboundp 'link-hint-copy-link)
         (autoload #'link-hint-copy-link "link-hint" nil t))
       (ignore
        (general-define-key "C-c l o" 'link-hint-open-link "C-c l c" 'link-hint-copy-link)))))

That looks to me like general-define-key is being called inside the eval-after-load 'link-hint no? So it won't run until link-hint loads, which it won't if there's an explicit :defer t and no other thing to autoload it?

[blaenk]

I forgot to include what :bind expands to, here it is. It seems like this one is correctly not being placed inside of eval-after-load, though I could be wrong.

(use-package link-hint
  :defer t

  :bind
  ("C-c l o" . link-hint-open-link)
  ("C-c l c" . link-hint-copy-link)

  :init
  (setq link-hint-avy-style 'post))

By the way notice the syntax I used in the earlier comment about how I used :bind was incorrect

(progn
  (progn
    (require 'package)
    (use-package-ensure-elpa 'link-hint))
  (unless
      (fboundp 'link-hint-open-link)
    (autoload #'link-hint-open-link "link-hint" nil t))
  (unless
      (fboundp 'link-hint-copy-link)
    (autoload #'link-hint-copy-link "link-hint" nil t))
  (condition-case-unless-debug err
      (setq link-hint-avy-style 'post)
    (error
     (ignore
      (display-warning 'use-package
                       (format "%s %s: %s" "link-hint" ":init"
                               (error-message-string err))
                       :error))))
  (ignore
   (progn
     (bind-key "C-c l o" 'link-hint-open-link nil nil)
     (bind-key "C-c l c" 'link-hint-copy-link nil nil))))

[noctuid]

You're right; I can replicate it now. It should be fixed though. This is what I get now:

(progn
  (unless
      (fboundp 'link-hint-open-link)
    (autoload #'link-hint-open-link "link-hint" nil t))
  (unless
      (fboundp 'link-hint-copy-link)
    (autoload #'link-hint-copy-link "link-hint" nil t))
  (condition-case-unless-debug err
      (setq link-hint-avy-style 'post)
    (error
     (ignore
      (display-warning 'use-package
                       (format "%s %s: %s" "link-hint" ":init"
                               (error-message-string err))
                       :error))))
  (ignore
   (general-define-key "C-c l o" 'link-hint-open-link "C-c l c" 'link-hint-copy-link)))

[blaenk]

Amazing! I'll wait for it to hit melpa to try it. Thanks so much!

[blaenk]

Hey sorry to be a bother but I found one hopefully last thing.

It seems like the way you decided to go with supporting wrappers doesn't mesh well when the key being bound isn't a string. For example, I had something like this:

  (use-package helm-describe-modes
    :general
    ([remap describe-mode] 'helm-describe-modes))

That previously worked, but once you added support for wrappers it seems to go into some infinite loop or something. I have to C-g to stop it. I wasn't sure what was causing this and had to bisect-comment out various packages until it became obvious that those where I bind things like this were causing problems.

For the record, I have another one that is similar which is like this:

(use-package irony
  :general
  (:keymaps 'irony-mode-map
   [remap completion-at-point] 'irony-completion-at-point-async
   [remap complete-symbol] 'irony-completion-at-point-async)

  :init
  (setq irony-user-dir (blaenk/cache-dir "irony"))

  (add-hook 'c++-mode-hook 'irony-mode)
  (add-hook 'c-mode-hook 'irony-mode)
  (add-hook 'objc-mode-hook 'irony-mode)
  (add-hook 'irony-mode-hook 'irony-cdb-autosetup-compile-options))

That one doesn't seem to cause any problems, presumably because the first argument is :keymaps and not some other thing.

[blaenk]

Also I'm not sure what other kinds of parameters kbd can take, or whatever it is that can accept both a string and a vector, so if there are other inputs it can take perhaps it would be good to account for those as well in whatever fix you come up with.

[noctuid]

Thanks for pointing this out; I probably wouldn't have realized it for a while otherwise. I hadn't considered that vectors (or something that evaluates to a string) could be mapped. It should be fixed now. Sorry about that; there shouldn't be anymore infinite loops even if the given argument list is malformed (previously there would be).

[blaenk]

Seems to be working now thanks a lot!

My only concern is, is there any kind of input that kbd or the underlying function can accept, other than a string and a vector? Perhaps a character or something? Does this fix account for that? In my case I only use these two things so it's not a problem for me, but perhaps it's something to think about.

[noctuid]

Basically to get the command names, I take the second item from every pair ignoring keywords. For the wrappers, the function name and positional arguments need to be removed from the list for this to work. I'm removing only symbols (that are not keywords) or things that are quoted. Neither of these are valid as the first argument to general-define-key, so there should not be any problems.

This wouldn't work if someone tried to use a variable containing a string though. I'm not sure if it's worth taking that case into account (and I can't think of any workaround).

[blaenk]

Sounds good to me.

[noctuid]

I'm actually going to open this back up.

Keys can also be bound to strings, so I shouldn't try to autoload strings.

I should decide whether I want to add something like :bind-keymap.

A better way of doing things that would still allow a variable containing a vector or string might be to record the names of the created definers and check against that.

[blaenk]

Yeah that sounds like it'd work. This problem seems to stem primarily from the way we went with using wrappers right? If we had used your other proposal of wrapper (sublist), i.e. where the wrapper is outside and before the list, this wouldn't be a problem right because the things would simply be passed-through to the wrapper/function, and general-define-key does something similar in that it passes through to kbd or whatever underlying bind function you use, which itself handles these different scenarios no? If that makes things easier I'm OK with switching to that or something else that disambiguates these situations.

[blaenk]

Personally I'm not a fan of the different :bind variants like :bind-keymap, in fact recent commits to bind-key show that there's an effort to unify things under the single :bind keyword. So if what my previous comment talks about further helps to negate the need for this, I'm even more inclined to switch to that.

In short I think we should prefer to pass-through as much as possible rather than replicate handling of special edge cases, especially if those cases change in the future we'd have to keep on top of that.

[noctuid]

this wouldn't be a problem right because the things would simply be passed-through to the wrapper/function

Yes, but it would have caused some other annoyances. I also like the current syntax better. I changed it so that it won't be an issue even if variables are used. I could always later add a list of wrapper names to have it work even in the case that the user is using their own positional argument wrappers (right now the builtin positional argument wrappers and anything defined with general-create-definer work though).

Personally I'm not a fan of the different :bind variants

Me neither. I won't add something like :bind*; the user can just specify the keymap, so it's not needed. :bind-keymap was just added in February and seems like it is necessary for its use case though. There is not a perfect way to infer whether a variable is an uninitialized keymap, so if I wanted to add support for binding a key to a keymap that hasn't yet been defined, I'd have to add a new keyword or some special syntax within :general.

In short I think we should prefer to pass-through as much as possible

I agree. I could potentially add some special syntax for binding a key to an uninitialized keymap and keyword to specify the package containing the keymap directly to general-define-key. I imagine it would be uglier than just using eval-after-load though. If you don't want/need :bind-keymap, I'll just not add this functionality unless someone else wants it.

[blaenk]

There is not a perfect way to infer whether a variable is an uninitialized keymap, so if I wanted to add support for binding a key to a keymap that hasn't yet been defined, I'd have to add a new keyword or some special syntax within :general.

Ahh okay. This is different from "binding a key on a keymap that hasn't yet been defined" right, since that's something that general already supports.

What if you can bind to (:keymap some-keymap) to specify that? I don't imagine that form could otherwise be valid?

[noctuid]

Yes I could do that if you want. I'd also need to add something like a :package keyword to specify the package the keymap comes from if I added this directly to general-define-key (it wouldn't be necessary in a :general section).

[blaenk]

That sounds good to me! But if you have reservations about it possibly getting out of hand then I understand. Personally I think it's a cool and simple way to use it (i.e. from the user's perspective).

[blaenk]

Hey perhaps I'm misunderstanding something but I've encountered some weird behavior with respect to the order in which the binds are created.

Basically as I mentioned in an earlier comment, I'm unbinding < and > in evil-normal-state-map so that I can use it as a prefix key for other commands, such as the ones I mentioned regarding smartparens or this one that's causing me problems now which is for evil-args (i.e. < a moves the current argument to the left).

With the smartparens bindings I added them using :keymaps 'emacs-lisp-mode-map :states 'normal and it works fine. With the evil-args binds I wanted to make them global in normal mode (i.e. :keywords 'normal) because want them to be global.

So in the :general section of (use-package evil ...) I unbind the <and > binds and in fact I also bind right after that < < and that works. Then in the :config section of (use-package evil ...) I add (use-package evil-args ...) which has a :general section which binds < a. You can find the full config here and what it macro-expands to here. That's what I've reduced the configuration to which still triggers the error:

(error "Key sequence > a starts with non-prefix key >")

So my reasoning was that the :general stuff happens before (even if right before) the :config section of the package, just like I imagined :bind worked (but I think I'm wrong on that), since it uses autoloads and stuff so there's no reason AFAIK to have to wait until the package loads? So by that line of reasoning I figured the unbinds would have taken effect by the time the :config section of the same package ran, which in this case contains the evil-args package configuration which then proceeds to bind on what I believed by-then would have been a prefix (i.e. <, for < a).

But instead it seems like the bindings are occurring after the entire :config section, throwing off my entire understanding of what occurs in what order.

I'm not saying that how I thought it worked is how it should work, but I'm wondering if perhaps you can help clear that up for me and how you would recommend I resolve this.

I made it work by adding this at the beginning of the :evil config:

(bind :keymaps 'normal
  "<" nil
  ">" nil)

Perhaps if the :general stuff ran right before :config (either because it waited for the package to load or because it didn't, as long as it ran before, although I'm not sure if there's a reason why it should wait until the package loads) then the unbinds I had in :general will have taken effect for whatever then ran in :config? I'm not sure.

This is the only time I've had to do an explicit general bind outside of a :general section. I know it probably sounds like I'm hellbent on having everything in :general, but it's more the question of, if it can be done then I might as well group it all together, and what may be causing this issue and if it may be resolved without too much trouble.

[blaenk]

I think perhaps that specific binding on :keymaps 'normal does have to wait for evil to load since evil-normal-state-map is defined by evil, so it wouldn't make sense to run before the package loads.

However, is there any reason it can't run at the very beginning of :config instead of after the entire :config section? That way I imagine the things inside :config can take advantage of that?

[noctuid]

so it wouldn't make sense to run before the package loads.

Yeah, it can't run until evil-normal-state-map exists, so it has to wait until evil is loaded. I guess when it runs, evil isn't loaded yet, so it has to defer the keybindings. At that point, I can't control when they get created relative to when the :config section executes. There may be a couple of things you could do since you aren't deferring evil. (require 'evil) before the use-package statement would ensure that the keybindings don't need to be deferred.

You could also maybe change the order of use-package-keywords, so that :general happens after :demand and :init but just before :config:

(setq use-package-keywords
      (cl-loop for item in use-package-keywords
               if (eq item :init)
               collect :init and collect :general
               else
               unless (eq item :general)
               collect item))

This would bring back the old problem if you explicitly used :defer t though (for other use-package statements if you didn't change the order back). The other possibility might be to not nest your use-package statements. Since you're not deferring evil, there's not really any reason to use its :config section for evil-args; you could just put that use-package statement below it. That would probably fix the problem. If you nest use-package statements a lot, you might also want to try req-package if you haven't already.

[blaenk]

Ah thanks. I previously didn't nest use-packages but I got the impression that it's the preferred way to do this kind of thing instead of using with-eval-after-load. AFAIK some of the evil packages need to be loaded after evil, and I previously did defer evil so I figured I'd do that.

I don't remember anymore why I no longer defer evil, I guess I'm not sure on what condition I'd defer it, since I want global (evil-mode 1), so I figure I just demand it load immediately.

Moving the nested packages out and after evil, and moving the unbinds back into the :general section, works out great. That way since I'm :demand t'ing evil, the use-packages after it will load after evil has loaded right? I ask because I think some (all?) of the evil packages have to be loaded after evil. It seems to work great though. I'm going to scour the rest of my configuration files to un-nest.

I think I previously nested because sometimes when I'd defer the outer package and the nested one needed to load after, I figured just add it to the :config section. There's :after for that though, AFAIK, so I suppose I can un-nest them and use :after. I've seen req-package but I think I'd prefer to use vanilla use-package if I can help it. use-package already has a lot of "magic" as it is.

[noctuid]

the use-packages after it will load after evil has loaded right?

Yes. If you don't want to use req-package you can continue to nest for deferred packages (or you could just put (require '<next package>) in the :config or something).

[blaenk]

Any updates on what you said here? Seems like a cool thing to introduce.

Perhaps every right-hand value gets rewritten into that, so that if you have regular (general-define-key "k" 'cmd), the 'cmd gets rewritten as something like '(:command 'cmd)), so that it can be treated uniformly. Then we can also have like I said there (:keymap projectile-command-map) and if necessary a :package to go with that for use outside of :general. Then it would be possible (and cool, though it's up to you) to expand that language for cool features which most users don't have to care about but which can be leveraged for additional behavior. This doesn't mean that we should go adding a billion features, but that it'd be nice to have the option.

For example hypothetically you could add a :which-key "some string" handler in case someone wants to set the description string for a key to appear in which-key. I think that would be amazing, but I'm not sure if you'd be inclined:

(general-define-key
  "C-x C-f" 'find-files)

(which-key-add-key-based-replacements
  "C-x C-f" "find files")

can be:

(general-define-key
  "C-x C-f" '(:command find-files :which-key "find files"))

Not that I am hell-bent on integrating which-key with general.el, just that it seems like it may be a cool little addition.

In fact this would be even better for prefix keys:

(which-key-add-key-based-replacements
  "C-c 8" '(:keymap iso-transl-ctl-x-8-map :which-key "unicode"))

[noctuid]

Yeah, I like the idea. I could make this user-extendable as well. The other day I was thinking it might be nice to be able to set the which key text for a prefix directly from a general-define-key, so I think I've changed my mind on that. This isn't really related to the original issue though, so let's have any further discussion in a separate one.

[jlee-made]

@blaenk Maybe you'd consider writing a blog post or something about this? I'm interested in moving gradually towards spacemacs-like bindings, and I imagine others are too -- and general looks like a great way to do it.

[blaenk]

Yeah I'm planning on writing a huge post about my overall move from vim to emacs, but that will take a while. I guess I can write a post about the many changes/additions to general.el that @noctuid has been gracious enough to indulge me with, but I'm not so sure if I'll do that anytime soon.

In the meantime, my emacs configuration is publicly available here. I imagine it's the only configuration out there at the moment that makes use of all of these new features since I'm the one that requested them and was testing them out. If you have any questions about it let me know!

Note that instead of general-define-key I use an alias called bind and a personal wrapper called my-map, defied in conf/common. That's all standard though thanks to general.el.