Refactored cursor and selection manipulation, and more! by derek-v-s · Pull Request #146 · elcritch/figuro

derek-v-s · 2025-05-17T08:43:40Z

Issue #145

textboxes.nim

Implemented cursor + anchor model for selection
Refactored almost all cursor movement procedures into shiftCursor
Created an Orient enum with enumerators for shiftCursor (e.g. Left, NextWord, TheEnd)
Created placeCursor procedure for direct placement by index
Created clearSelection procedure
Created growSelection procedure
Rewrote delete and created deleteSelected
Rewrote insert, cursorUp, cursorDown, selection=, and findLine
Organized the cursor and selection procedures
Wrote documentation for various procedures
Changed NextWord / PreviousWord functionality to always jump to the beginning of each word

input.nim

Ensured the cursor blinks
Created activate() procedure
Rewrote delete-previous-word logic
Switched word skipping to Meta+Arrow-keys (this is much more common)
Switched word selection to Meta+Shift+Arrow-keys
Eliminated cursor alignment problem by connecting layoutResize slot

tinput.nim

Ensured input is activated, and the cursor is visible on init
Ensured cursor is placed at TheEnd on init

tinput2.nim

Ensured input is activated, and the cursor is visible on init
Ensured all numbers are editable

ttextboxes.nim

Updated tests based on new model / procedures

So I'm finally, mostly confident that everything works. It might be hard to believe I spent many hours every day for a full week on this. There's so much to understand and consider (as you already know).

There is, however, at least one remaining issue to deal with. Originally the insert procedure wouldn't allow any text to be added beyond what was originally set, when in Overwrite mode. That means it couldn't be used for the common insert/overwrite toggle feature. I've rewritten it to allow for that now, but that means that in tinput2 you can write numbers past the established range (00:00:00). So it seems like we need another option that can be set, such as LimitRange. Let me know if you have any ideas about that, and of course anything else in this PR.

Issue elcritch#145 textboxes.nim - Implemented cursor + anchor model for selection - Refactored almost all cursor movement procedurs into shiftCursor - Created an Orient enum with enumerators for shiftCursor (e.g. Left, NextWord, TheEnd) - Created placeCursor procedure for direct placement by index - Created clearSelection procedure - Created growSelection procedure - Rewrote delete and created deleteSelected - Rewrote insert, cursorUp, cursorDown, `selection=`, and findLine - Organized the cursor and selection procedures - Wrote documentation for various procedures - Change NextWord / PreviousWord functionality to always jump to the beginning of each word input.nim - Ensured the cursor blinks - Created activate() procedure - Rewrote delete-previous-word logic - Switched word skipping to Meta+Arrow-keys (this is much more common) - Switched word selection to Meta+Shift+Arrow-keys - Eliminated cursor alignment problem by connecting layoutResize slot tinput.nim - Ensured input is activated, and the cursor is visible on init - Ensured cursor is placed at TheEnd on init tinput2.nim - Ensured input is activated, and the cursor is visible on init - Ensured all numbers are editable ttextboxes.nim - Updated tests based on new model / procedures

elcritch · 2025-05-17T19:33:55Z

So I'm finally, mostly confident that everything works. It might be hard to believe I spent many hours every day for a full week on this. There's so much to understand and consider (as you already know).

Totally believable – I've spent so many hours dealing with subtle details with font layout, grid layout, events, etc. It's pretty rewarding though working on it at the fundamental level! But it can be a slog at times.

I'm looking forward to giving it a whirl!

There is, however, at least one remaining issue to deal with. Originally the insert procedure wouldn't allow any text to be added beyond what was originally set, when in Overwrite mode. That means it couldn't be used for the common insert/overwrite toggle feature.

Sweet! Did you happen to add tests for that as well? I wanna make sure we test those so refactoring or bug fixing is easier in the future. They're a pain but the LLMs are pretty good at writing them.

I've rewritten it to allow for that now, but that means that in tinput2 you can write numbers past the established range (00:00:00). So it seems like we need another option that can be set, such as LimitRange. Let me know if you have any ideas about that, and of course anything else in this PR.

Fine by me, and yeah generally we'll need some sort of configurable input filters. I think HTML input filters might be the best model.

It wasn't clear to me what the best way to add them would be. As you probably saw, there's the raw key inputs, and then input creates a second set of higher level events. That makes sense. What's unclear now is how best to have overrides that the end-user widget be can intercept and modify or cancel the high level events. Then again having a callback or regex could be another approach, as signals and slots don't have direct returns.

haha, yep you can see what I've been working on for a couple of years. I wanted to get the fundamentals set well, even if not perfect now so they can evolve. Also, trying to bridge the best between HTML and QT/GTK is an interesting challenge. It wasn't an original goal of Figuro but has evolved into that. ;)

elcritch · 2025-05-17T19:42:21Z


-    if not connected(self, doKeyCommand, self, keyCommand):
-      connect(self, doKeyCommand, self, Input.keyCommand)
-    if not connected(self, doUpdateInput, self):


I like moving this to initialize, though it goes back to the question of how best to add a filter. One way is this if not connected(self, doUpdateInput, ...) which would allow the user of Input to connect their own doUpdateInput handler first. I never followed through trying it out though, but did add the is connected logic in Sigils for this sort of scenario.

Dropping it for now makes sense though. Just wanted to point it out to you.

I forgot to ask about this. What's the difference between the initialize slot and using onInit in the draw slot?

I'm not clear about how the slot override by users works in terms of timing. Are you saying that connecting things in this initialize slot will block users from overriding those connections?

I forgot to ask about this. What's the difference between the initialize slot and using onInit in the draw slot?

Generally speaking initialize is used when implementing a widget, while onInit is for customizing an existing widget like Input when it's instantiated.

I added initialize first, but it's more hassle to add, so I added onInit as well. Technically, doInitialize & initialize are a signal and slot, while onInit is part of the the preDraw callback.

Hmmm, maybe I should make a diagram. I'm also thinking instead of preDraw calling that stage setup?

Widget Lifecycle

Widget Allocated and preNode internal API runs

Widget preDraw callback is called (e.g. Setup) -- this is what the user adds

Widget doDraw signal emitted that calls draw and other slots.

Widget postDraw callback is optionally called (unused currently)

Widget destroy (not implemented yet, I think)

I'm not clear about how the slot override by users works in terms of timing. Are you saying that connecting things in this initialize slot will block users from overriding those connections?

Hmmm, yes I think that would as initialize runs first. However, it's not clear what is the best way to override the doInput signals. I also experimented with a separate overrideInput or something.

Suggestions are welcome!

How does this look?

https://github.com/elcritch/figuro/blob/f78059fcc0ae010376f9cd5b76cac6d5924791f2/README.md

All the details probably don't need to be there, but I don't know what's useful to you or others.

What does the preNode step refer to? And what's the condition were initialize is skipped? When the user has used onInit?

Do you mind asking that at #147 ?

elcritch · 2025-05-17T19:43:06Z

                this.updateInput(rune)
-                this.text.cursorNext()
+            onInit:
+              this.activate()


Nice, so the onInit use cases is making sense now?

elcritch · 2025-05-17T19:44:53Z

      discard
  elif down.matches({KAlt, KShift}):
    case pressed.getKey
-    of KeyLeft:


Hmmm, do we loose the alt-left alt-right handling of did you move it elsewhere? It'd need to be refactored to handle ctrl-left on windows/linux anyways.

The word skipping and word selecting are Ctrl (Meta) based instead of Alt now. This is how things usually are set up on Windows/Linux. It's been a while since I've used macOS so I can't recall how it usually works on it.

Okay, yeah MacOS uses alt. But that can be changed later.

Ah ok. Yea, this is something to have configurable by users anyhow.

Ah ok. Yea, this is something to have configurable by users anyhow.

Yeah, that'd be best. The current setup doesn't allow that aside from configuring Meta.

elcritch · 2025-05-17T19:57:28Z

haha, yep you can see what I've been working on for a couple of years. I wanted to get the fundamentals set well, even if not perfect now so they can evolve. Also, trying to bridge the best between HTML and QT/GTK is an interesting challenge. It wasn't an original goal of Figuro but has evolved into that. ;)

BTW to clarify this point, I mean the signals and events and core objects.

Being able to make it easy to use yet extensible is an interesting challenge. QT, GTK and the rest always seem to "hardcode" it into a big object hierarchy while HTML makes it almost impossible to customize. I also like Mac's Cocoa design where the input handler can be overriden at runtime. I used it to run Vim or Emacs plugins for years.

derek-v-s added the ready for review label May 17, 2025

elcritch reviewed May 17, 2025

View reviewed changes

Comment thread src/figuro/ui/textboxes.nim Outdated

elcritch reviewed May 17, 2025

View reviewed changes

Comment thread src/figuro/widgets/input.nim Outdated

elcritch added 2 commits May 18, 2025 03:18

Update src/figuro/ui/textboxes.nim

314f5e3

Update src/figuro/widgets/input.nim

06dbf00

elcritch merged commit e44eaad into elcritch:main May 18, 2025
2 checks passed

derek-v-s removed the ready for review label May 18, 2025

Conversation

derek-v-s commented May 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elcritch commented May 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Widget Lifecycle

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

elcritch commented May 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

derek-v-s commented May 17, 2025 •

edited

Loading