`Delete(Sub)?Word` does not respect selection ranges

[past-gen]

Description

In short, when InSelection — it treats the End/Start of the selection, for Right/Left direction respectively, as the current cursor position, and additionally yoinks the adjacent symbols, instead of deleting what's between selection ranges.

• For better understanding:

  wezterm-gui_NgjDZsYS4H.mp4

As you can see, after Undoing, it shows the actual area that was affected. Utterly irritating and highly inconvenient.

In my opinion, selection should always take priority in such cases, as the user has consciously chosen to interact with that specific area and this should be respected. Regardless of the navigation mode.

UPD.1

Alternatively, an additional action called DeleteInSelection is needed, at least. So it could be easily combined as DeleteInSelection|Delete(Sub)?Word(Left/Right) per user preference.

Assuming the current behavior is intentional, of course. With TUI apps, you never know what's a bug and what's a feature really...

UPD.2

My sloppy attempt at solving this locally

• plug/custom/main.lua

  function preventHastyDeletion(bp)
    local cursors = bp.Buf:GetCursors()
    local selection
    for c = #cursors, 1, -1 do
      c = cursors[c]
      if c:HasSelection() then
        c:DeleteSelection()
        selection = true
      end
    end
    return selection
  end
  

• /bindings.json

  {
    ...
    , "Alt-Backspace"      : "lua:custom.preventHastyDeletion|DeleteWordLeft"
    , "OldBackspace"       : "lua:custom.preventHastyDeletion|DeleteSubWordLeft"
    , "Shift-OldBackspace" : "lua:custom.preventHastyDeletion|SelectToStartOfLine&Backspace"
    , "Alt-Delete"         : "lua:custom.preventHastyDeletion|DeleteWordRight"
    , "Delete"             : "lua:custom.preventHastyDeletion|DeleteSubWordRight"
    , "Shift-Delete"       : "lua:custom.preventHastyDeletion|SelectToEndOfLine&Delete"
    ...
  }
  

It does pretty much what I need, although I still think that this should be part of the core logic. Also, not sure if this is the most optimal solution possible.

---

Environment

• Version: 2.0.16-dev.15 ( 3a7403b )
• OS: -
• Terminal: -

[JoeKar]

@masmu
Could this be a task for you, since you're most experienced in this direction (looking at #2665, which is related to this)?

[past-gen]

since you're most experienced in this direction

@masmu I've another question regarding the (Sub)?Word logic. It's not exactly part of the current issue, although somewhat related.

Would it be possible to grant higher priority to the quotes [`"']+ over other \W symbols?

As an example: when trying to delete/navigate through RegEx "(?s)(?:some|ThInG).*$", it currently yoinks "(? and ).*$", but would be much preferable if it were split into " <= (? <= ... and ... => ).*$ => " respectively.

With quotes being considered a "wrapping" symbol, from my perspective it makes sense. However, I can't be sure that it is technically feasible without affecting overall performance.

Wanted to open a separate [FR] for this, but think it'd be better to hear your opinion first.

[masmu]

@JoeKar I'd be happy to.

@past-gen is absolutely right. The actions should behave as they do in other text editors.

Would it be possible to grant higher priority to the quotes [`"']+ over other \W symbols?

I am open to it as I think it would be a nice addition. Please open a new issue and lets continue the discussion over there and gather the opinions of others.

[past-gen]

I am open to it as I think it would be a nice addition

Glad to hear! Opened #3988 , please take a look.

[past-gen]

@masmu Regarding the 5e6bc3a, what about cases where the user adds additional cursors with the mouse while only the primary one has a selection? Or if the user spawns additional cursors from a selection and then adds more with the mouse?

wezterm-gui_rt5PJhqbmh.mp4

Correct me if I'm wrong, but what you're suggesting is to respect an existing selection per-cursor, but if there's none - select something yourself ( i.e. fallback to its default behavior ).

If so, this would only make the situation worse, as it could lead to unpredictable outcomes. Or rather, predictable, but undesirable ones.

A more modest approach would be more fitting here I think, whereby cursors without a selection are to wait patiently until there' no more selections present.

[masmu]

I understand your point of view here.
The observed behavior stems from the Delete*Word actions being registered in MultiActions, which causes the event loop to execute the action strictly once per cursor in isolation. This prevents the "wait patiently" behavior, which requires awareness of the global selection state across all cursors.
We can address this by removing the four actions from the MultiActions map and handling cursor iteration internally with a function like:

func (h *BufPane) DeleteSubWordRight() bool {
	cursors := h.Buf.GetCursors()
	hasSelection := false
	for _, c := range cursors {
		if c.HasSelection() {
			hasSelection = true
			break
		}
	}

	for _, c := range cursors {
		h.Buf.SetCurCursor(c.Num)
		h.Cursor = c
		if hasSelection {
			if h.Cursor.HasSelection() {
				h.Cursor.DeleteSelection()
				h.Cursor.ResetSelection()
			}
		} else {
			h.SelectSubWordRight()
			if h.Cursor.HasSelection() {
				h.Cursor.DeleteSelection()
				h.Cursor.ResetSelection()
			}
		}
	}
	h.Relocate()
	return true
}
}

While this resolves the interactive issue effectively, it introduces a change in behavior for Lua plugins. Previously, bp:DeleteSubWordRight() operated on a single cursor. With this change, calling this method from a plugin will execute the logic for all cursors simultaneously. Plugin authors would lose the ability to invoke this specific high-level action on a single cursor selectively.
If demand is high enough, we could introduce new actions that do not need to ensure backward compatibility rather than changing the established behavior for existing ones.

[past-gen]

I... don't even know what to say here. A tad-bit impractical implementation, innit?

If one tiny applied change can easily crumble the whole logic, maybe it is as good a reason to consider its full reimplementation from scratch, as any?

It may be a good idea to introduce new methods or improve existing ones, such as Cursor.(De)?Select*, so it's possible to update and detect the current selection state globally from the buffer. Not in detail, but at least the current amount in the pane, since this seems to be lacking at the moment.

And maybe a more modal approach could be taken this time so that it won't affect handlers designed for plugins/bindings unnecessarily, while remaining easily modifiable at a core level.

Also, navigation, selection and deletion shouldn't be tied to each other. Combined? Yes. But not tied.

A couple of additional ifelse blocks surely won't solve anything at this point, especially when considering #3988 in addition to the current issue.

[JoeKar]

Or rather, predictable, but undesirable ones.

In regards to #3987 it is predictable now and generic.

If one tiny applied change can easily crumble the whole logic, maybe it is as good a reason to consider its full reimplementation from scratch, as any?

Maybe we do, after we processed the tons of left open issues and things with a higher priority (at least from our perspective), for which we spend our free time. 😉

When I understood you and your first post (incl. updates) right, then you prefer to perform the action on the selection only in case there is one and if there is none proceed e.g. deleting according further actions.

So from my perspective it looks like your UPD.1 seems to be the best compromise solution:
We add the additional Delete(In)?Selection, which then can be chained as you like.