Refactoring selection and caret manipulation in textboxes.nim

[derek-v-s]

While working on understanding the existing code, to implement selection-by-mouse, I was compelled to start refactoring based on the inclusion of the idea of an anchor point (where the selection starts). I'm not done yet, but I hope you will agree that things already seem easier to understand. I've also eliminated some duplicate code.

Before

proc cursorLeft*(self: var TextBox, growSelection = false) =
  if growSelection:
    if self.selection.len() == 1:
      self.growing = left
    case self.growing
    of left:
      self.selection = self.selWith(a= self.clamped(left, offset = -1))
    of right:
      self.selection = self.selWith(b= self.clamped(right, offset = -1))
  else:
    self.selection = toSlice self.clamped(self.growing, offset = -1)

proc cursorRight*(self: var TextBox, growSelection = false) =
  if growSelection:
    if self.selection.len() == 1:
      self.growing = right

    case self.growing
    of left:
      self.selection = self.selWith(a= self.clamped(left, offset = 1))
    of right:
      self.selection = self.selWith(b= self.clamped(right, offset = 1))
  else:
    # if self.selection.len != 1 and growing == right:
    self.selection = toSlice self.clamped(self.growing, offset = 1)

After

proc growSelection*(self: var TextBox) =
  self.selectionRange.a = min(self.caretPos, self.anchor)
  self.selectionRange.b = max(self.caretPos, self.anchor)
  if not self.selectionExists: self.selectionExists = true
  self.updateSelection()

proc clearSelection*(self: var TextBox) =
  self.anchor = self.caretPos
  self.selection = self.caretPos .. self.caretPos
  if self.selectionExists:
    self.selectionExists = false
    self.updateSelection()

proc shiftCaretHorizontally*(self: var TextBox, direction: LeftOrRight) =
  let n: int = case direction
              of right: self.caretPos + 1
              of left:  self.caretPos - 1
  self.caretPos = clamp(n, 0, self.runes().len())
  self.designCaret()

[elcritch]

That does look cleaner! A PR would be great so I could review it a bit more.

[derek-v-s]

Coming up soon. Trying to make sure everything still works. 😅

[elcritch]

Coming up soon. Trying to make sure everything still works. 😅

Any tests you wanna add in that pursuit would be awesome! ;) There should already be some tests for the text box.

[derek-v-s]

One of the things preventing me from finishing this is a lot of flip flopping about procedure names and parameters, and how much to conflate into one procedure. For example, right now I have:

proc shiftCursor*(self: var TextBox,
                              direction: LeftOrRight,
                              select = false) =
  # Shifts the keyboard cursor one position left or right
  # based on the enumerators: left or right.
  # Clears the selection and brings the anchor along unless
  # select is set to true.
  let n: int = case direction
              of right: self.cursorPos + 1
              of left:  self.cursorPos - 1
  self.cursorPos = clamp(n, 0, self.runes().len())
  self.designCursor()
  if select: self.growSelection()
  else: self.clearSelection()

I was originally going to do shiftCursorHorizontally and shiftCursorVertically, but now I'm favoring just having shiftCursor and enumerators for left, right, up, and down.

Does this make sense to you?

And what do you want do you want to do about an enumerator convention? Right now we are lacking consistency, and I'm not sure what I prefer. So for directions I guess the choices are something like Direction.Left, dirLeft, Left, or left, where the first two are in alignment with the standard lib guidelines. Do reasons exist for adopting those guidelines? What would you prefer as a user?

[derek-v-s]

I just looked at Qt's docs and they utilize what I'm leaning toward, with the addition of how many times to perform the operation.

https://doc.qt.io/qt-6/qtextcursor.html#movePosition

bool QTextCursor::movePosition(QTextCursor::MoveOperation operation, QTextCursor::MoveMode mode = MoveAnchor, int n = 1)

Moves the cursor by performing the given operation n times, using the specified mode, and returns true if all operations were completed successfully; otherwise returns false.

[derek-v-s]

I'm thinking an enum named Orient, leading to either Orient.Left / Orient.Start / Orient.NextWord or o_Left / o_Start / o_NextWord (embrace the power of the dark_side) 😸 .

[elcritch]

I'm thinking an enum named Orient, leading to either Orient.Left / Orient.Start / Orient.NextWord or o_Left / o_Start / o_NextWord (embrace the power of the dark_side) 😸 .

Enum works for me. No underscores though 🤣 Unfortunately I've got a mix of enums with and without prefixes. :/ I'd say without now that 2.0 works better with them.

Also I'd say copy QTs naming if possible. Though Forward/Back/Start or Next/Prev, or similar might be better. Remember some languages are left-to-right or even top-to-bottom. It's better to have names not hard coding left/right.

Also it'd be nice to easily expand to next sentence, prev sentence, next paragraph, Prev paragraph as well. That could be another enums, or additions in the Orient enum.

[elcritch]

Oh the QT one makes sense, MoveOperation makes more sense than Orient. Though I don't care too much as Orient could be be seen as an "adverb" to a move signal which is the name.

I'd say we should just copy all the QT options and no-op and provide not implemented warnings. Then other folks might get an itch to implement it. ;)

[derek-v-s]

What do you think about discoverability? The reason I was thinking of Orient.NextWord or o_NextWord is that you would get a clean reference to the full list via an editor's code completion feature. Otherwise the person has to consult documentation until they memorize all of them, versus just remembering Orient. or o_. I know there's some aversion to underscores in the nim community, but it seems like an easy way to set enumerators apart, and make them seem less like gibberish than say moNextWord, which would also include anything else starting with mo in the code completion list.

[derek-v-s]

The right-to-left and top-to-bottom languages are something I hadn't considered. I can imagine that the logic for things like NextWord, EndofLine, Left, or Right would have to handle these cases, but it seems like the names still work. It seems like switching to forward/back would introduce the problem you are imagining. If right-arrow key is bound to shiftCursor(Forward) then "forward" would be left in a right-to-left language, and down in a top-to-bottom language. So the right-arrow key would have unexpected behavior in those cases. Maybe I'm missing something.

[derek-v-s]

I chose Orient because "move" or "shift" is already part of the function name. We are then orienting the movement toward something like NextWord or EndofLine.