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

Already on GitHub? Sign in to your account

Blank commands interspersed between steps #66

Closed
magnars opened this Issue Jul 19, 2012 · 10 comments

Comments

Projects
None yet
2 participants
Contributor

magnars commented Jul 19, 2012

This fails:

When I insert "text here"
And I go to point "0"
And I press "M-d"
And I press "C-f"
And I press "M-d"
And I press "C-y"
And I press "M-y"
Then I should see "text"

with the error Previous command was not a yank

This, however, works:

When I insert "text contains"
And I go to point "0"
And I press "M-d"
And I press "C-f"
And I press "M-d"
And I press "C-y M-y"
Then I should see "text"

Which does the yank and yank-pop in one step.

Adding a post-command-hook in env:

(add-hook 'post-command-hook #'(lambda () (message "this-command: %S, this-original-command: %S"
                                                  this-command this-original-command)))

reveals the blank command that is interspersed:

this-command: nil, this-original-command: nil
this-command: forward-char, this-original-command: forward-char
    And I press "C-f"

which of course makes testing yank-pop and other things reliant on last-command less than intuitive.

Contributor

rejeep commented Jul 19, 2012

Hi,

This is actually more of an espuds issue, but you don't need to move it. I understand it's hard for people to see the difference sometimes.

When I implemented the pressing keys stuff, I tried what you do in your first example, but I never got it to work. Not sure why... This is why action chains exists (see https://github.com/rejeep/espuds/blob/master/espuds-input.el#L11-27). Action chains are not pretty and makes no sense if you read a scenario, so I would very much like to solve the issue.

The problem is that espuds get something like C-y in on step. Then some stuff happens in ecukes which intercepts. Then espuds get M-y. The action chain adds to a list and execute the complete list when told to.

I have been thinking about a solution to this, which is to be lazy. So for each key that is pressed, espuds checks if the key is bound to an action or if it's a prefix key. If it's a prefix key, the key is added to a buffer. Next time, what is in the buffer is concatenated with the key pressed. If it's bound to a function, it is executed. The problem with this though is in these cases:

When I press "..."
And I do something else
When I press "..."

To solve this I think ecukes have to remember the last command which espuds can check each time a key is pressed.

I see no simple and good solution I'm afraid. If you have any, please do let me know!

Contributor

magnars commented Jul 19, 2012

I know we've talked about this before, and it seems I'm having a hard time wrapping my head around it. :-)

Wrapping it in an action chain works for sure. But I still don't get what ecukes is doing that gives the emacs command system a nil-command? What would be the detriment of wrapping all my "When-and"-blocks in an action chain?

Maybe it's time for me to dig into the source code for ecukes.

Contributor

rejeep commented Jul 19, 2012

Not sure exactly when Emacs set these variables, but for each step the function ecukes-step-run (https://github.com/rejeep/ecukes/blob/master/ecukes-run.el#L34) is executed. So it has to be something in that function or the function it calls.

Contributor

rejeep commented Nov 18, 2012

I have actually no idea why this happens. But a simple solution would be to do this in Espuds:

(When "^I press \"\\(.+\\)\"$"
      (lambda (keybinding)
        (when (and
               (equal espuds-previous-keyboard-input "C-y")
               (equal keybinding "M-y")
               (eq (key-binding (kbd "M-y")) 'yank-pop))
          (setq this-command 'yank))
        (let ((macro (edmacro-parse-keys keybinding)))
          (if espuds-chain-active
              (setq espuds-action-chain (vconcat espuds-action-chain macro))
            (if (and (equal keybinding "C-g")
                     (eq (key-binding (kbd "C-g")) 'keyboard-quit))
                (espuds-quit)
              (execute-kbd-macro macro))))
        (setq espuds-previous-keyboard-input keybinding)))

Whatcha think?

Contributor

rejeep commented Nov 20, 2012

I implemented this feature in the yank-yank-pop-fix branch (https://github.com/rejeep/espuds/tree/yank-yank-pop-fix). Works for me, can you try it out?

Contributor

magnars commented Nov 25, 2012

Hmm, I might be doing something wrong. I headed into util/espuds and did a git fetch and git co origin/yank-yank-pop-fix, then ran the tests again. They still ran. Then I changed

And I press "C-y M-y"

to

And I press "C-y"
And I press "M-y"

And now I get Previous command was not a yank ... so either it didn't work, or I'm using a wrong version ecukes to work with the espuds fix, or I did something other stupid.

Contributor

rejeep commented Nov 25, 2012

FYI, since this is an Espuds issue, I will fix it as a part of the espuds milestone (https://github.com/rejeep/ecukes/issues?milestone=2&state=open)

Contributor

rejeep commented Nov 25, 2012

Hmm... not sure why that is. It works for me if I run with Ecukes branch v0.3 and Espuds branch ecukes-v0.3.

Contributor

magnars commented Nov 25, 2012

Yeah, I'm not over on v0.3 ecukes yet, so guess that's it.

Contributor

rejeep commented Nov 25, 2012

Allright, I'm closing this and when v0.3 is out, try it out and open this issue again if it doesn't work.

@rejeep rejeep closed this Nov 25, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment