Rewrap bounds feature

[AndreasArvidsson]

This is an alternative and simpler solution for the rewrap bounds problem. Instead of adding an additional rewrap action I have just defined it so that if you wrap bounds it turns into rewrap/replace instead.
round wrap bounds air

fixes #270

[pokey]

It's a clever idea, but I have mixed feelings. I don't like that we lose expressiveness in case for whatever reason someone does want to wrap the bounds, especially as we're planning to generalise the notion of bounds. Also not super intuitive to me because "round wrap bounds" seems to pretty clearly indicate that we're going to wrap the bounds.

What's wrong with just having a "rewrap" command? I don't think it would be any more complex than what you've done here. Could just have the context object contain information about the boundary ranges and read that in the "rewrap" action. The context for surrounding pairs is constructed in one place so it should be quite straightforward

[AndreasArvidsson]

There is still an escape hatch if we actually want to wrap the bounds. take bounds round wrap this
But I think that is so unlikely that you want to do that that I don't think it's a problem.

There's nothing wrong with it per se but I have 3 reasons for prefer this implementation

1. rewrap recognized really badly for me
2. Adding additional actions when we can make do with clever implementations of existing ones is not something I favor in general. I find it quite intuitive that if you wrap something that is already wrapped it will actually change it. Kinda like our format action which first removes existing format.
3. The implementation was very easy since we already have the bounds as our selection. We have a current limitation where we can only have one modifier per command so we can't default to bounds when someone says round rewrap state. This could of course be fixed but the current implementation was the path of least resistance.

[pokey]

There is still an escape hatch if we actually want to wrap the bounds. take bounds round wrap this But I think that is so unlikely that you want to do that that I don't think it's a problem.

There's nothing wrong with it per se but I have 3 reasons for prefer this implementation

1. rewrap recognized really badly for me

Yeah that doesn't surprise me, but I don't think is a strike against the action itself. We can find another name, eg "change" or "switch" or "rebox" or "repack"

2. Adding additional actions when we can make do with clever implementations of existing ones is not something I favor in general.

Not sure I agree with that philosophy. I'd rather something be dumb and obvious then clever and surprising 🙂

I find it quite intuitive that if you wrap something that is already wrapped it will actually change it.

That's not what this does, though, right? And it is actually useful to be able to wrap something that is already wrapped. I do that fairly commonly. Something is in square brackets and I want to parenthesize it, etc

Kinda like our format action which first removes existing format.

3. The implementation was very easy since we already have the bounds as our selection.

I think that implementing "rewrap" will be even easier. As suggested, just drop the info you need on the context object when we construct it, and read it from the "rewrap" action. No need for the clever logic you have here

We have a current limitation where we can only have one modifier per command so we can't default to bounds when someone says round rewrap state. This could of course be fixed but the current implementation was the path of least resistance.

Not sure why you'd want to say "round rewrap state"? Also, we can just add a simple modifier preference of surroundingPair: any, and then we could say things like "round rewrap air", and that would infer "round rewrap pair air", or "round rewrap square", which would override the default preference

If you don't feel strongly about the philosophical argument and would just prefer to have an action you can use today, I'd be happy to impl "rewrap" today or tomorrow. If it turns out to be more complicated than I thought then let's revisit this approach?

If, on the other hand, you feel strongly about the philosophical argument, let's hop on a discord and hash this one out; I don't think it should be too difficult to find something we're both happy with here

[AndreasArvidsson]

I don't feel strongly about it if you do please go ahead with another implementation.

[pokey]

Alright, calling my bluff; I like it 😄. Give me a day or two and I'll report back