consecutive narrowing

[blaenk]

I love this package. Would it be possible to make it so that if I choose a prefix command from helm-descbinds, it'll close helm-descbinds and leave emacs in a state where it's waiting for the next key?

For example the scenario I'm imagining is:

1. Press C-x
2. Wait a bit, emacs shows "C-x -" at the bottom
3. Press C-h to trigger helm-descbinds, choose C-x 4 which is a prefix command
4. Descbinds should close and I should be back where I was
5. After a bit, emacs shows "C-x 4 -" at the bottom
6. I can press C-h again to trigger descbinds if necessary, now narrowing from the point of "C-x 4"

I saw which-key which looks pretty cool, and lets you do this (same with guide-key, which it's an improvement on AFAIK). I much prefer helm-descbinds though, by far, but this is the functionality I sorely miss.

[michael-heerdegen]

That sounds useful.

But is it really helpful to be able to leave the Helm session in between? Because I think it would be easier to implement that when you select a prefix key, you get the narrowed session instantly without quitting helm. Doing otherwise might also be confusing.

[blaenk]

Yeah I suppose that would be perfectly fine.

[michael-heerdegen]

Jorge Israel Peña notifications@github.com writes:

Yeah I suppose that would be perfectly fine.

Ok, good.

Currently, the behavior for the main action on a prefix key is not useful at all (in my tests, just nothing happened).

So I'll try to just change that. Should not be hard, AFAICT, all we need is already there.

[michael-heerdegen]

I did exactly that now, via action-transformer attribute, and bumped the version to 1.10.

Please give it some testing.

[blaenk]

Thanks.

One thing I've noticed is that when I go say 2 levels deep into a prefix, descbinds disappears and only reappears once I press a key, though sometimes (not always) it comes back on its own after a delay:

I believe this sequence would replicate it:

1. C-x C-h
2. pattern: 8 prefix
3. RET
4. pattern: ' prefix
5. RET
6. helm-descbinds disappears. it comes back if I press a key, but the key is also output to the file buffer
7. pattern: A
8. RET
9. properly inserts the character

I'll let you know if I find anything else.

[michael-heerdegen]

I can't reproduce - but you may have hit a problem I faced in other situations.

In 6., does Emacs display any text in the minibuffer ("RET -" maybe)?

You could also try to (if you know how to load lisp code) substitute the run-with-idle-timer in my commit with just run-with-timer.

[blaenk]

Nope the minibuffer doesn't show anything after step 5 actually, that's weird.

Ok I opened the helm-descbinds.el, made the change to run-with-timer, did M-x eval-sexp, and re-ran the steps I outlined. Indeed that seems to have resolved it; it seems to work perfectly. Any chance we can go with that function instead of the idle one? Or does this better narrow where my problem lies?

The character was properly inserted, but now I do see a "RET -" in the minibuffer once the character was inserted (at step 9 above) and descbinds exited. It seems like the final RET both did the descbinds action as well as passed RET over to emacs separately.

[blaenk]

I'm on 24.5.1 if that matters.

[michael-heerdegen]

I see no problem with run-with-timer, seemingly your Emacs is not always idle after leaving Helm and that caused the problem. I'll just make the change. I think it's a separate issue, different from the "RET - " thing.

I see the "RET -" thing here and there all the time when exiting Helm. I guess it's a subtle thing that has to with our usage of minor-mode-overriding-map-alist or something like that. discard-input doesn't seem to be able to break it.

I this case here, it's only a visual effect (the "second" RET does not have any effect), but in some situations it had (e.g. I hit "a" then and got "RET-a is undefined" or something like that.)

[michael-heerdegen]

Ok, please check if it's ok now.

[blaenk]

I think you forgot to bump the version? My emacs doesn't detect any updates for the package. Anyways, that's the change I used myself when you told me to try it, so I think it's safe to say it'll work again :)

[blaenk]

My mistake, I was on melpa anyways, so version doesn't matter. I noticed that it sees the update now. I tested it and it does work :) I'll let you know if anything comes up, but I suppose it's fine to close this now if you want!

[michael-heerdegen]

Closing. Please reopen if you should find a problem.

Thanks for the idea!

[blaenk]

I just noticed that this doesn't seem to be working anymore.

[michael-heerdegen]

Did the latest change suggested by Thierry fix the problem? The update should be available on Melpa soon.

[blaenk]

It doesn't seem like it. When I select a prefix key helm-descbinds goes away as if I chose an actual key sequence, whereas I believe IIRC it should re-open but narrowed to that prefix (so only those keys beginning with that prefix are shown).

[michael-heerdegen]

Yes, that what it's supposed to do. It works well for me. Can you try to increase the 0.1 in "helm-descbinds-action-transformer" and see if that helps? Thanks.

[blaenk]

To what? I set it to 0.9 and it didn't make a difference.

I instrumented it and am stepping through it in EDebug and I noticed that the condition for re-running descbinds keeps failing. It checks to make sure that (equal (cdr-safe cand) "Prefix Command"), but it's showing for me that the result of that cdr-safe is "Prefix\ Command". I tried changing the string that's compared to to "Prefix\ Command" and "Prefix\\ Command" and it didn't solve it. I printed out the (cdr-safe cand) and when printed out it does look like "Prefix Command" so I am misunderstanding something.

Sorry I forgot to note that I'm on emacs 25, although I thought I remembered this happening on 24 as well but I could be mistaken. If you don't have the time to look into it on 25 no worries.

[blaenk]

When I change the condition to t so that it always executes, it does work out perfectly. So it looks like the condition is the problem.

[blaenk]

Here's the backtrace when inside that function:

  (equal (edebug-after (edebug-before 2) 4 (cdr-safe (edebug-after 0 3 cand))) "Prefix Command")
  (if (edebug-after (edebug-before 1) 5 (equal (edebug-after (edebug-before 2) 4 (cdr-safe (edebug-after 0 3 cand))) "Prefix Command")) (edebug-after (edebug-before 6) 7 (list (cons "helm-descbinds this prefix" (function (lambda (cand) (edebug-enter (quote edebug-anon95) (list cand) (function ...))))))) (edebug-after 0 8 actions))
  helm-descbinds-action-transformer(nil ("C-x" . Prefix\ Command))
  helm-get-actions-from-current-source()
  helm-execute-selection-action-1()
  helm-execute-selection-action()
  helm-internal(<MY BINDINGS REDACTED; TOO LARGE>)
  describe-bindings()
  funcall-interactively(describe-bindings)
  call-interactively(describe-bindings nil nil)
  command-execute(describe-bindings)

So it looks like it's passed in as a symbol or something? Not a string. Changing it to:

(equal (cdr-safe cand) 'Prefix\ Command)

Makes prefix commands work correctly.

However, I just realized that regular commands (even without the above fix) don't actually work. I guess I hadn't noticed because I usually use descbinds to explore bindings. When I choose a non-prefix key I get this message:

Error running timer: (user-error "Key sequence contains no complete binding")

[blaenk]

Disregard that last part, that was something to do with evil.

So yeah the fix I guess is to check if it's equal to the string "Prefix Command" or the symbol 'Prefix\ Command".

[blaenk]

Here's what I changed it to, but perhaps you can come up with something better:

--- /home/blaenk/.dots/emacs/.emacs.d/elpa/helm-descbinds-20160608.1225/helm-descbinds.el
+++ #<buffer helm-descbinds.el>
@@ -230,7 +230,7 @@
 (defun helm-descbinds-action-transformer (actions cand)
   "Default action transformer for `helm-descbinds'.
 Provide a useful behavior for prefix commands."
-  (if (equal (cdr-safe cand) "Prefix Command")
+  (if (memq (cdr-safe cand) '("Prefix Command" Prefix\ Command))
       `(("helm-descbinds this prefix" . ,(lambda (cand)
                                            (run-with-timer
                                             0.1 nil

[michael-heerdegen]

Jorge Israel Peña notifications@github.com writes:

However, I just realized that regular commands (even without the above fix) don't actually work. I guess I hadn't noticed because I usually use descbinds to explore bindings. When I choose a non-prefix key I get this message:

Error running timer: (user-error "Key sequence contains no complete binding")

What exactly do you mean? Does this happen whenever you hit a prefix key (I guess not, that would be very insane), or what regular commands did you actually try, and how?

Emacs 25 here too, btw.

Do you have any addons installed that could cause this trouble?

[blaenk]

Nah as I mentioned in a recent comment that was a false alarm, sorry for the confusion.

I provided a potential fix in my most recent comment. Let me know what you think.

[michael-heerdegen]

Jorge Israel Peña notifications@github.com writes:

I provided a potential fix in my most recent comment. Let me know what you think.

It's ok I think. But note that the package itself does output the symbol named "Prefix command" (in you case at least), namely via "helm-descbinds-transform-candidates". We try with "intern-soft" whether a symbol with that name exists. Then we return that symbol, else, just the string.

That works because normally there is no interned symbol existing named "Prefix command". But it seems in your case it is. But I'm quite sure that "helm-descbinds" did not initially create that symbol ("intern-soft" never creates a symbol).

Anyway, we should not just assume that this interned symbol doesn't exist, so we should install a fix similar to yours. I would prefer to prevent soft-interning in "helm-descbinds-transform-candidates" for this case.

[blaenk]

Yeah I'm not sure why it's defined. I have other key related packages like which-key and general.el, perhaps one of those do it.

[michael-heerdegen]

Jorge Israel Peña notifications@github.com writes:

Yeah I'm not sure why it's defined. I have other key related packages like which-key and general.el, perhaps one of those do it.

Ok, it's not verboten anyway... I'll make the change in 12h or so, must go to bed now. Thanks for investigating!

[blaenk]

No problem, thanks!

[thierryvolpiatto]

Jorge Israel Peña notifications@github.com writes:

Disregard that last part, that was something to do with evil.

So yeah the fix I guess is to check if it's equal to the string "Prefix Command" or the symbol 'Prefix\ Command".

+  (if (memq (cdr-safe cand) '("Prefix Command" Prefix\ Command))

This test i.e memq is using eq:

(memq "Prefix Command" '("Prefix Command" Prefix\ Command)) => nil

(member "Prefix Command" '("Prefix Command" Prefix\ Command)) => ("Prefix Command" Prefix\ Command)

(member 'Prefix\ Command '("Prefix Command" Prefix\ Command)) => (Prefix\ Command)

HTH, don't know if it is related to your problem.

Thierry

[blaenk]

Ah thanks. It was working with memq but I guess member is more correct.

[thierryvolpiatto]

So this patch is working fine for me:

@@ -232,5 +232,7 @@
-  (if (equal (cdr-safe cand) "Prefix Command")
-      `(("helm-descbinds this prefix" . ,(lambda (cand)
-                                           (run-with-timer
-                                            0.1 nil
-                                            #'describe-bindings (kbd (car cand))))))
+  (if (member (cdr-safe cand) '("Prefix Command" Prefix\ Command))
+      (helm-make-actions
+       "helm-descbinds this prefix"
+       (lambda (cand)
+         (run-with-timer
+          0.1 nil
+          #'describe-bindings (kbd (car cand)))))

The helm-make-actions is unrelated to the fix it just avoid backquotes (which are correct) making the patch more readable.

[thierryvolpiatto]

Also it seems the use of timer is creating a bug: Hit C-x C-h, select C-x r press RET and then press C-g. now press again C-x, you can see in minibuffer C-g C-x, so if now you press C-h, describe-bindings will try to complete on C-g C-x which is probably not that we want.

Removing the timer works fine for me here:

@@ -232,5 +232,5 @@ Provide a useful behavior for prefix commands."
-  (if (equal (cdr-safe cand) "Prefix Command")
-      `(("helm-descbinds this prefix" . ,(lambda (cand)
-                                           (run-with-timer
-                                            0.1 nil
-                                            #'describe-bindings (kbd (car cand))))))
+  (if (member (cdr-safe cand) '("Prefix Command" Prefix\ Command))
+      (helm-make-actions
+       "helm-descbinds this prefix"
+       (lambda (cand)
+         (describe-bindings (kbd (car cand)))))                 

[michael-heerdegen]

Jorge Israel Peña notifications@github.com writes:

Ah thanks. It was working with memq but I guess member is more correct.

It worked for you because you are always in one and the same case where you are testing the symbol. For all other, it would fail.

[michael-heerdegen]

I've merged Thierries approach -thanks @thierryvolpiatto - and bumped the version.

[blaenk]

Thanks @thierryvolpiatto !