Paredit kill (Draft PR)

[dandavison]

Hi @whitphx, thanks very much for your extension: I've been a heavy user for a couple of years now.

This is a work-in-progress PR adding paredit-kill. For me, for a very long time (15+ years), this has been the paredit command I've used the most. It's bound to ctrl+k in Emacs paredit. I use it with any language, e.g. Python, Rust, Javascript, Scala, C, Bash, etc, in addition to Emacs Lisp.

This PR isn't quite finished: it doesn't yet behave exactly as the Emacs paredit command. But I made the PR a few weeks ago, and I've been using it every day ever since. I'm opening the PR because it is already useful, and just in case anyone else wants to work with me on fixing the remaining bugs (and adding tests).

Here's the Emacs documentation for paredit-kill:

Kill a line as if with ‘kill-line’, but respecting delimiters.
In a string, act exactly as ‘kill-line’ but do not kill past the closing string delimiter.
On a line with no S-expressions on it starting after the point or within a comment, act exactly as ‘kill-line’.
Otherwise, kill all S-expressions that start after the point.

These screenshots show some examples of regions that would be killed by the command:

(Those screenshots were created using this commit: c2776fb967f4d2e30f2440a63a76c5a7d7f0c570)

And here are some other resources documenting and explaining the command:

http://danmidwood.com/content/2014/11/21/animated-paredit.html https://github.com/emacsmirror/paredit/blob/d0b1a2f42fb47efc8392763d6487fd027e3a2955/paredit.el#L1409 https://github.com/emacsmirror/paredit/blob/d0b1a2f42fb47efc8392763d6487fd027e3a2955/paredit.el#L353

[dandavison]

I'll fix those test failures.

[dandavison]

Hi @whitphx, the tests should pass now. The yank portion of the tests is commented out, since that was not passing (I'm not sure why yet -- yank does appear to work as expected in interactive usage.)

Here is an example of something that does not work correctly yet:

  const initialText = `(
  (
    a b
  )
  (
    c d
  )
)`;

With cursor here: const initialText = <CURSOR>`(, the result is

  const initialText = `;

So as you can see it incorrectly leaves a backtick. In Emacs, in contrast, it deletes both backticks and the material contained within them:

  const initialText = ;

There are several other scenarios where results differ from Emacs in a way that is obviously not correct.

[dandavison]

Would you mind kicking off the CI run here @whitphx?

[whitphx]

@dandavison Hi, thank you for your great work, and sorry that I could not answer quickly. I permitted the CI.

It seems that the CI is broken now regardless of this PR, and we can consider the result OK if the tests on Windows passed.

[dandavison]

Hi @whitphx, thanks! I really appreciate the work you've put into this extension -- it's fantastic.

The tests did pass on Windows, but I do want to emphasize that there are still situations where this feature does not kill text correctly (and those don't have unit tests). E.g. the one described in the comment above: https://github.com/whitphx/vscode-emacs-mcx/pull/1425#issuecomment-1274640555. I'll add remaining bugs like that to this PR (or an issue) as I encounter them.

So, of course, it's completely up to you how close to bug-free you want this to be before merging it.

[dandavison]

Here's an incorrect case (it should kill to end of line in this situation, but the shading shows that it actually kills to the end of the next line)

"source": source_name,
{"id": f'{target_name}.{space}' if space else target_name},
op

[dandavison]

And another one, in Scala. Again, this should kill to end of current line only:

      val userIdInQueryParams = 
      if (satisfiesCustomAccessControlLogic(ctx.key.opaqueName1)) {

[whitphx]

@dandavison

So, of course, it's completely up to you how close to bug-free you want this to be before merging it.

I think it's OK if these kinds of bugs are introduced as it does not break any existing features. So, I will follow your opinion. Please mark this PR as ready for review when you think you have implemented it at your desired level 👍

[whitphx]

The CI problem may have been fixed at the main branch, so can you merge the current HEAD and push it to this PR branch when you do further works? thanks.

[dandavison]

Hi @whitphx, I have still not fixed all the bugs but it's been a few months now and I still use this many times every day. If it's OK with you then perhaps we could merge this. It's just possible that having it merged will lead to someone else being inspired to help fix the remaining bugs.

[whitphx]

@dandavison Thank you very much for such a huge contribution! As suggested, I would like to release and merge it :) Could you please create an issue describing the remaining bugs?