AmaiKinono / puni

Structured editing (soft deletion, expression navigating & manipulating) that supports many major modes out of the box.
GNU General Public License v3.0
403 stars 21 forks source link

Puni-slurp-forward does not work when current sexp is two level deep and empty #33

Closed Inc0n closed 2 years ago

Inc0n commented 2 years ago

Puni-slurp-forward does not work if the current sexp is two level deep and at the end of both sexp.

| indicates where the cursor is at.

(for (block (in-list blocks))
  (for (inst (in-list (cdr block)))
    |))
inst

where it would does nothing at the position shown above, The following would work however.

(for (block (in-list blocks))
  (for (inst (in-list (cdr block)))
    )|)
inst
AmaiKinono commented 2 years ago

I guess what you want is

(for (block (in-list blocks))
  (for (inst (in-list (cdr block)))
    |
inst))

? That's a great idea.

Inc0n commented 2 years ago

Oh sorry, yes ... well something like:

(for (block (in-list blocks))
  (for (inst (in-list (cdr block)))
    inst|))
AmaiKinono commented 2 years ago

Should be fixed. Please test.

Inc0n commented 2 years ago

Nice, thank you :)

It does exactly as you have demonstrated in your snippet, therefore, cames with that extra of line of space as well 😃

AmaiKinono commented 2 years ago

cames with that extra of line of space

To me this is expected. The sexp after ) starts from the first char after ), which is a newline, and ends at the last char of inst.

Inc0n commented 2 years ago

I see now, thanks for the explanation.

Inc0n commented 2 years ago

@AmaiKinono, I must apologize first for not realising the extend of this feature request, now that I would also prefer to have the original functionality to remain accessible, whilst co-exist with this newly added feature.

I realise this is addional complexity to be introduced, so of course I would like to hear your decision on this matter.

AmaiKinono commented 2 years ago

to have the original functionality to remain accessible, whilst co-exist with this newly added feature.

Could you elaborate on this and explain why is it preferred?

Inc0n commented 2 years ago

Right, so the new multi level slurp is prefered for reasons listed already.

The original intent comes in handy when I was doing something similar to:

(let ((bind-name value|)))
some-other-exp

As per usual, | indicates the cursor.

What I would like to do is to slurp some-other-exp into the let body, but not into the level where the cursor is at right now, which is how the newest slurp behave.

This is where the conflict of interests arise. Well, either way I feel like this should be made aware of as it is from my suggestion that this is made impossible with the current slurp.

AmaiKinono commented 2 years ago

We could do this in 2 ways. We can just move the cursor before slurping:

(let ((bind-name value))|)
some-other-exp

Or we can change the behavior of Puni so slurping only moves one delimiter at a time:

(let ((bind-name value|)))
some-other-exp
;; slurp
(let ((bind-name value|))
some-other-exp)
;; slurp again
(let ((bind-name value|)
some-other-exp))
;; ...

So which is better? To me the first one looks better but I don't have a strong reason.

Inc0n commented 2 years ago

Haha, now that you lay them out in such a way, the first one is also more appealing to me as well.

AmaiKinono commented 2 years ago

OK, I'll close the issue then.