calva icon indicating copy to clipboard operation
calva copied to clipboard

Deleting top-level form makes cursor jump to end of file

Open jelmerderonde opened this issue 3 years ago • 5 comments

Recently I’m having a problem whenever I delete a top-level form using ctrl+alt+backspace. The form is correctly deleted, but also the cursor jumps to the end of the file.

For now I'll just add the issue here, but will try to debug further using the instructions here: https://github.com/BetterThanTomorrow/calva/wiki/How-to-Hack-on-Calva

jelmerderonde avatar Jul 14 '22 14:07 jelmerderonde

Thanks for reporting! I can reproduce this bug.

Before:

a|
b
c

Calva: Kill/Delete Sexp Backwards

Expected:

|
b
c

Result:


b
c|

It happens in paredit.ts/killRange: https://github.com/BetterThanTomorrow/calva/blob/d829fbaaff516f7f5add86d11d4804b412fed3c9/src/cursor-doc/paredit.ts#L16-L26

Meaning we have the problem with these Paredit commands:

  • paredit.killRight
  • paredit.killSexpForward
  • paredit.killSexpBackward (the command reported on)

I have some little time for this now, @jelmerderonde, so will have a look and possibly attempt to fix it. Holler if you have already started.

PEZ avatar Jul 16 '22 10:07 PEZ

I was wrong about paredit.killSexpForward. It seems to work. I'm guessing it's about the range passed to killRange. Struggling a bit with how to expose this in unit test...

PEZ avatar Jul 16 '22 10:07 PEZ

Hmmm, these tests pass:

    describe('killRange', () => {
      it('Deletes top-level range with backward direction', async () => {
        const a = docFromTextNotation('a |<|b |<|c');
        const b = docFromTextNotation('a |c');
        await paredit.killRange(a, textAndSelection(a)[1]);
        expect(textAndSelection(a)).toEqual(textAndSelection(b));
      });
      it('Deletes top-level range with forward direction', async () => {
        const a = docFromTextNotation('a |>|b |>|c');
        const b = docFromTextNotation('a |c');
        await paredit.killRange(a, textAndSelection(a)[1]);
        expect(textAndSelection(a)).toEqual(textAndSelection(b));
      });
      it('Deletes nested range with backward direction', async () => {
        const a = docFromTextNotation('{a |<|b |<|c}');
        const b = docFromTextNotation('{a |c}');
        await paredit.killRange(a, textAndSelection(a)[1]);
        expect(textAndSelection(a)).toEqual(textAndSelection(b));
      });
      it('Deletes nested range with forward direction', async () => {
        const a = docFromTextNotation('{a |>|b |>|c}');
        const b = docFromTextNotation('{a |c}');
        await paredit.killRange(a, textAndSelection(a)[1]);
        expect(textAndSelection(a)).toEqual(textAndSelection(b));
      });
    });

PEZ avatar Jul 16 '22 11:07 PEZ

This actually has nothing to do with killRange. It happens when formatting the current range while at the top level:

Before:

a
|
c

Calva: Kill/Delete Sexp Backwards

Expected:

a
|
c

Result:

a

c|

PEZ avatar Jul 16 '22 11:07 PEZ

After some more digging, it seems to be a regression upstream in VS Code. It also seems fixed in VS Code Insiders, bringing hope that it will be fixed in the next VS Code update. Though I fail to find the issue or commits where this is addressed, so I can't link to anything interesting.

PEZ avatar Jul 16 '22 13:07 PEZ