BetterThanTomorrow / calva

Clojure & ClojureScript Interactive Programming for VS Code
https://marketplace.visualstudio.com/items?itemName=betterthantomorrow.calva
Other
1.68k stars 217 forks source link

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

Open jelmerderonde opened 2 years ago

jelmerderonde commented 2 years ago

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

PEZ commented 2 years ago

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:

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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.