Proposal to Improve Window Focus Management with Minimal Changes

nicholascarroll commented 3 months ago

Hey I'm working on independed window scrolling in the switchWindows branch of my emsys fork. So that when you have two multiple windows on the same buffer you can scroll in one without the others also scrolling. And I thought "how nice it would be to have focusWin up there on editorConfig just like focusBuf?"

So....

The Idea

Let's introduce a setWindowFocus function, use an underscore prefix for the focused field in our editorWindow struct, and add a focusWin pointer to our editorConfig. So then you can use E.focusWin and reduce code a little.

Proposed Changes

Update the editorConfig struct:


struct editorConfig {
   // ... other fields ...
   struct editorWindow **windows;
   struct editorWindow *focusWin;  // New field
   int nwindows;
   struct editorBuffer *focusBuf;
   // ... other fields ...
};

Update the editorWindow struct:

struct editorWindow {
   // ... other fields ...
   int _focused;  // Underscore suggests "don't touch directly"
   // ... other fields ...
};

Add a setWindowFocus function:

void setWindowFocus(struct editorConfig *config, struct editorWindow *window) {
   if (config->focusWin) {
       config->focusWin->_focused = 0;
   }
   config->focusWin = window;
   window->_focused = 1;
   config->focusBuf = window->buf;
}

Update any direct modifications of focused to use setWindowFocus instead. This is basically only when switching windows, creating or destroying windows.
Keep direct reads of _focused (like if (E.windows[i]->_focused)) as they are for simplicity and performance. And not bother with a getter.

Architectural Considerations

Config, focused buffer and focused window are actually singleton.

Impact of Not Doing It

just a little bit more code everywhere. It's just a convenience.

Code changes Required

the editorWindow focused property is only currently set in 3 places:

InitEditor
editorSwitchWindow
case DESTROY_WINDOW

Everything else can stay the same.

What Do You Think?

If this looks good, I will prepare a PR! 😊

CC @ScoreUnder

japanoise commented 3 months ago

Looks good to me! Would be fine with enhancing the window focusing code to make it easier to use. Something I've toyed with on other projects is storing the cursor position and scroll state in the window, rather than in the buffer; that way you can open a new window with its own mark and cursor.

By the way, Score isn't an active maintainer here at the moment - no need to ping her for approval, she just helped me out early on in emsys development with a segfault I was having.

nicholascarroll commented 3 months ago

Cool. So I'll do that next.

I have implemented independed window scrolling in my branch windowScroll, and the way I did it is:

The buffer cursor remains the source of truth and drives everything
struct editorWindow now has cx,cy, which are for bookmarking the buffer cursor position when switching away from that window. Only used for switching.
screen cursor, which is scx, scy, is migrated from the buffer to the window struct. So that is the displayed cursor and its only used for rendering. So there isn't a bidirectional translation between coordinate systems. Not needed, except when I get up to doing word wrap (visual-line-mode), then I will have to put in some special logic for that.
The row offset rowoff is migrated from buffer to window. I have also added coloff for horizontal scrolling when I do toggle-truncate-lines.
Rendering functions are now editorWindow struct based instead of editorBuffer based.
Screenrows and Screencols are understood to mean the total terminal height and width. The window struct has its own height. Width.....since side by side windows is planned for never, then window width is the same thing as screencols.

Now another disreputable design idea I have is: if this text editor is never intended to get alot more than bare bones functionality, then quite a few functions don't actually need arguments. For example, editorScroll will only ever conceivably be scrolling the currently focused window. Most stuff to do with edits and cursor movements as well.

nicholascarroll commented 3 months ago

I'm closing this and backing out the pull request because I'm not convinced its actually an improvement. While it looks nicer to me as a beginner, it does create two ways of doing things.

japanoise / emsys