DarwinAwardWinner / with-simulated-input

Test your interactive elisp functions non-interactively!
GNU General Public License v3.0
38 stars 4 forks source link

with-simulated input doesn't work with transient.el #16

Closed Trevoke closed 2 years ago

Trevoke commented 2 years ago

Hi and thank you for this package! it's a lifesaver when working with code that doesn't separate UI from logic well, which is most of org-mode code :D

I'm trying to write an acceptance test around my package. with-simulated-input was perfect when I had read-multiple-choice but I am trying to use transient.el and it seems to not register input in a way that's friendly with your package. I asked a question here - https://emacs.stackexchange.com/questions/69534/acceptance-test-for-a-flow-that-uses-the-transient-package

but then I figured you might have a unique understanding of the situation and it might be faster to reach out to you -- it might be something that with-simulated-input is currently not equipped to handle?

sample code:

(require 'transient)
(require 'with-simulated-input)

(defun ogt-print-a () (interactive) (insert "a"))
(defun ogt-print-b () (interactive) (insert "b"))

(transient-define-prefix ogt-transient ()
  ["X"
   ("f" "test f" ogt-print-a)
   ("g" "test g" ogt-print-b)])

(with-simulated-input "f"
  (ogt-transient))

When I run this code, it displays the transient, and if I try to hit f myself, the transient says it's in an inconsistent state :/

DarwinAwardWinner commented 2 years ago

Hi! There does indeed appear to be something weird going on here. Unfortunately, I don't know enough about transient.el and how it works internally to say where the problem lies. What I can tell you is that at its heart, with-simulated-input ultimately uses execute-kbd-macro to do its work. You should try to reproduce the same behavior directly with execute-kbd-macro without involving with-simulated-input and see if the problem still happens. That will probably give us more information to work with.

Trevoke commented 2 years ago

If you mean doing this (same code as before except for actual execution):

(ogt-transient)
(execute-kbd-macro "f")

then when I use eval-buffer on this, it properly inserts the a.

DarwinAwardWinner commented 2 years ago

I was thinking more along the lines of binding ogt-transient to a key and then using execute-kbd-macro to send that key followed by "f". However, the fact that what you did actually works gives me an idea as to what is wrong. I will test it and get back to you shortly.

Trevoke commented 2 years ago

I did the following to bind ogt-transient to a key, which also worked:

(defvar stag-transient-test-mode-keymap (make-sparse-keymap))

(define-minor-mode stag-transient-test-mode "" :keymap stag-transient-test-mode-keymap)

(define-key stag-transient-test-mode-keymap (kbd "C-c d j") #'ogt-transient)

(stag-transient-test-mode 1)

(execute-kbd-macro (kbd "C-c d j f"))
DarwinAwardWinner commented 2 years ago

Ok, so I think the problem is that ogt-transient doesn't actually read any input at all. Running (ogt-transient) just creates the transient buffer and sets up the key bindings, and then it returns. From that point, the key bindings are handled by the normal Emacs event loop. However, with-simulated-input expects that the lisp forms in BODY will be reading input directly, e.g. via read-string. I'm not entirely sure yet why this break things, but I'm pretty sure it has something to do with this.

Trevoke commented 2 years ago

That makes sense. Thanks for exploring so quickly. And I think you're right, I was looking at the source for transient.el and https://github.com/magit/transient/blob/master/lisp/transient.el#L1893 adds pre-command hooks and post-command hooks, so I guess it's working at a very low level of emacs.

For the short term, thank you for helping me find a way to get unblocked with my tests - and for the long term, do let me know how I can help further, and whether you think this is outside of the scope of with-simulated-input to handle or not :)

DarwinAwardWinner commented 2 years ago

Ok, I understand why it doesn't work now. When with-simulated-input executes BODY, as soon as BODY returns a value, that value is returned from with-simulated-input, and any remaining input is discarded. So ogt-transient returns, and that's the end of the evaluation because, as mentioned above, it returns without reading any keys. Hence, the "f" is never used. As for why it ends up leaving transient in an inconsistent state, I see that like with-simulated-input, transient also let-binds and manipulates overriding-terminal-local-map. It appears that these two packages' manipulations of this keymap are not mutually compatible. To fix this, I believe I would need to redesign with-simulated-input to avoid let-binding overriding-terminal-local-map. I will try to do so when I have time, but in the meantime, you have a viable solution with execute-kbd-macro.

DarwinAwardWinner commented 2 years ago

To be clear, this is actually 2 problems:

  1. ogt-transient doesn't work by reading input directly, which violates the assumption of with-simulated-input.
  2. transient and with-simulated-input both manipulate overriding-terminal-local-map in conflicting ways.

Problem 1 is solved by simply not using with-simulated-input. Problem 2 explains why you're getting the inconsistent state warning. I'm going to use this issue to track Problem 1, which we have already solved, and I will open another issue for Problem 2.

Trevoke commented 2 years ago

Makes perfect sense, thanks!

DarwinAwardWinner commented 2 years ago

Unfortunately, I don't think this is a useful place to report this, as the behavior you describe has nothing at all to do with with-simulated-input, so reporting it here is not going to get this fixed. If anything, this sounds like a bug in transient.el and should be reported to the issue tracker for that package, where the author can provide the proper support.

publicimageltd commented 2 years ago

Sorry I had deleted the post, but I was too slow; github already sent it to you. :face_with_head_bandage:

I actually found out that execute-kbd-macro has to include a final key which quits, then it works.

Thanks for your patient answer!