Issues with key sequences

[johannox]

I think I stumbled upon two issues in ShoSho that I couldn't get resolved. There might be an issue in ShoSho, or I might use it wrong.

1. I think it should be possible to add both single key shortcuts, as well as key sequence shortcuts that contain the same keys; however, I couldn't get this to work
2. ShoSho seems to trigger sequences again, even if only the last key is pressed (without the sequence before)

I couldn't get ShoSho to run with fava, so I added some very basic bun/happydom tests to illustrate these issues.

[fabiospampinato]

It took a while to take a look at this, but now I have.

First of all I can't run the tests with bun, it just stays stuck here:

First test

The first test is basically this:

test ( 'key sequences do not trigger single key shortcuts', async () => {
  let f_pressed = 0;
  let g_pressed = 0;
  let fg_pressed = 0;

  const sho = new ShoSho ();
  sho.register('f', () => { f_pressed++; });
  sho.register('g', () => { g_pressed++; });
  sho.register('f g', () => { fg_pressed++; return true });
  sho.start()

  keypress('f')
  expect(f_pressed).toEqual(1)

  await new Promise(resolve => setTimeout(() => resolve(''), 1000));

  keypress('g')
  expect(g_pressed).toEqual(1)
  expect(fg_pressed).toEqual(0)

  keypress('f')
  keypress('g')
  expect(f_pressed).toEqual(1)
  expect(g_pressed).toEqual(1)
  expect(fg_pressed).toEqual(1)
})

Going step by step:

keypress('f')
expect(f_pressed).toEqual(1)

We trigger f, and f_pressed is incremented, makes sense.

We wait 1 second, that doesn't change anything, shosho doesn't care about timings like that.

Then we do this:

keypress('g')
expect(g_pressed).toEqual(1)
expect(fg_pressed).toEqual(0)

We trigger g, and g_pressed is incremented, makes sense too. fg_pressed is not incremented because f was already handled before.

Lastly we do this:

keypress('f')
keypress('g')
expect(f_pressed).toEqual(1)
expect(g_pressed).toEqual(1)
expect(fg_pressed).toEqual(1)

In some sense I see where you are coming from, f and g got triggered within a microtask, what we really want in this scenario is fg, not f and g. The thing is that this scenario doesn't actually exist in practice, 2 events are not going to be triggered within the same microtask like that ever, plus the user is going to physiologically need some milliseconds between the two keypresses.

So basically the first test is wrong, shosho is working as expected here, it's just not time-sensitive like that, so the two scenarios, with and without the 1s delay, are identical to it.

Second test

First we do this, and it works as expected:

keypress('f')
keypress('g')
expect(fg_pressed).toEqual(1)
expect(fh_pressed).toEqual(0)

Then we do this, and it also works as expected:

keypress('f')
keypress('h')
expect(fg_pressed).toEqual(1)
expect(fh_pressed).toEqual(1)

Lastly we do this:

keypress('g')
keypress('h')
expect(fg_pressed).toEqual(1)
expect(fh_pressed).toEqual(1)

And fg gets actually triggered. This is obviously a bug in shosho. Now how to fix it is not obvious, unfortunately what looked like pretty reasonable code months ago now looks like arabic almost 🤣 The error is that we are not properly resetting the state of the currently possibly active non-komani keypresses. I think this is where the bug occurs:

shosho/src/index.ts

Lines 236 to 240 in 28c00ed

	if ( !triggered || isMouseEvent ( event ) || chord & CODE2ID.MetaLeft || chord & CODE2ID.MetaRight ) { // Meta keys are buggy, they will swallow keyup events for the trigger
	this.chords = triggered ? [this.chords[index] &~ id] : [0n];
	}

The thing is I don't remember exactly why those lines look like that, I'm going to need to look at this again tomorrow with a fresher mind. Right now I'm thinking that if the chord got handled we should always reset the current list of chords with a this.chords = [0n], but the code very obviously does something different than that, and I'd be inclined to trust my past self that there is a reason for it, so the fix may be more subtle.

In general we could definitely benefit from some automated tests, this stuff is tricky to test so for now I've been doing some manual tests. The thing is also that I want to see the library working if a switch to different layouts, those kinds of tests would be a nightmare to automate.