-
Notifications
You must be signed in to change notification settings - Fork 48
Add ability to resize individual views from Lua and add an event for view resizing. #655
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: default
Are you sure you want to change the base?
Conversation
Previously it was not possible to set the size of a view that was the second child of a split to a fixed size because it was not possible to know the size of its parent. This commit amends the view metatable to allow Lua to get and set the size of individual Scintilla widgets. It also adds a "resize" event that is called whenever a Scintilla widget changes sizes and passes a pointer to the associated view table to the handler function. This allows Lua user scripts to "pin" a view to a specific size regardless of window and parent split size changes by defining a handler function that resets the size of a view on change.
|
I got the GTK build working on MacOS again (had to re-add the keys['cmd+l'] = function ()
ui.print(ui.command_entry.height)
endAnd watch the output be a non-zero size even when the command entry is hidden. Even more strange, if you expand the command entry box before hiding it, the above print will show the expanded size even though It occurs to me that since the view.width and view.height logic is working for views below the top one, refactoring the command window to be a standard view as I mentioned above would solve this issue. But maybe a more experienced set of eyes will figure out what I missed. |
|
Thanks for taking the time to put this together. I'll take a look when I have some time and get back to you. |
|
Thanks. Right now I'm working on a couple Lua modules that demonstrate the usefulness of this functionality so it has a reason to exist (Scintilla statusbar, drop-in command entry replacement) but once I have a couple working PoCs I intend to circle back around to the UI quirks. |
|
I just made a small revision to the way resize events are handled that I haven’t had time to test and commit yet, should be coming later today. |
|
phew... It took me way longer than I thought to familiarize myself with all the nuances of programmatically altering text, view focus, and how UI events are emitted, but I finally have a working proof of concept that demonstrates why this is valuable: Presenting the "Textbar" Module That was stealing all my time from checking to make sure that the build wasn't broken by my changes, now that I have that in a somewhat working state I'll see if I can get those changes pushed tonight so this is ready for review. |
I realized that instead of having a wrapper function just to get access to lua_pushdoc, I could add a couple of fake "types" for emit() to pick up on, so that we can just directly push SciObject and document pointers directly onto the stack from emit. I feel like this simplifies things. There's also some code in here where I was goofing around with trying to get GTK to allow me to resize the command entry to less than 15 pixels tall, but so far nothing is working...
|
Alright @orbitalquark, I think this is finally ready to review now. I'm sure you'll have questions and want to refactor this in your own style, but I think the addition of view.width, view.height, and events.RESIZE make some interesting functionality possible. It's working (more or less) on all 3 platforms. |
|
Let's suppose By the way, we shouldn't define new |
I'm learning both GTK and Qt as I go, so I'm not 100% sure of the answer here, but from my research both Paned and QSplitter objects are designed to be user resizable, rather than app managed. Where a box layout would automatically resize to the minimum widget size, my understanding is that the split widgets leave size up to the user, requiring us to resize manually from user code. Setting the minimum size will just prevent the splitter from making the window any smaller, which is not necessarily what we want if we want to preserve user control
I did that in my initial commit, but the problem is that |
I see. Usually with widget sizers, we can have some widgets as shrinkable, and others as growable. If we say the shrinkable one has a min width of 200, then the growable one should auto-size until it's as large as can be (the shrinkable one is 200). And by auto-size, I mean we'd probably call some sort of sizer function that recomputes child geometries. It's something to keep in mind, but you may very well be right that splitters are user-controlled.
We should probably declare something in src/textadept.h like |
|
That's exactly what I had in my initial attempt #8089db4, so if that strategy seems better to you then that's fine. I think having the "fake" types in src/textadept.h might save a few lines of code but I didn't actually check. It's your call. There are a bunch of ways this could be implemented, I think I was aiming for backwards compatibility. |
|
Ah, I see that now. Yes, that is the preferred approach. |
|
Thanks again for submitting this. I've had a look at this and I have some concerns (and sorry in advance if I missed something in your previous set of comments):
I'm going to propose the following:
I do not plan on creating a module that implements something like If you have any thoughts about this, let me know. We can always iterate to try and get to something satisfactory. |
|
I really appreciate you taking the time to look at this and consider it.
That makes sense, my only concern is the overhead of calling
To be honest, there are plenty of ways to easily crash Textadept with the event handlers we have now. I have triggered a number of stack overflows by connecting a handler to That being said, I wasn't completely satisfied with how setting
Fair enough, I mostly did that because I couldn't get GTK to behave, and I was playing with QSplitter collapse behavior.
Agreed.
Yeah, I didn't realize that until after I'd proposed the bigger restructure of panes/views. I can see how adding any more features at this point poses a challenge.
I think that will be sufficient. As I mentioned in another thread, I found a way to hack around fetching size (move the split beyond its maximum position and then query the value) but I don't know if it's good practice to rely on that and it could cause ugly flickering if you're constantly flipping sizes.
I really like this, even if nothing else comes of this I think
I think that's fair to keep the core lean. I know it takes effort to review these changes so I appreciate the thought put into this. I've given my feedback above, I'll respect whatever you decide. |
|
Fair point about event shenanigans. If we have a view resize event, we should probably have a window resize event too. Or at least have one event that encompasses both. I will look into this. |
|
I was experimenting with a resize event, and GTK is giving me trouble. When I use the command entry to run |
I think I ran into this when I was testing the GTK build, but I was hoping it was just a "GTK is kind of broken on MacOS" issue. I wonder what the correct incantation is to prevent GTK's event loop from getting stuck. I know you've already spent a lot of time on this and I don't want it to hold up 13.0. It seems like renaming the size field and adding width and height attributes to the split table gets us 90% of the way there, and adding the events could be something for a minor release since that won't be a breaking change. |
I had mentioned my intention to write this in the comments on #649, but it ended up being a lot more than the 20 LoC I had projected. I wanted to get this out there before 13 hits release since it seems like something that would be included in a major release. Since @orbitalquark likes to merge changes manually I wrote this so that it wouldn't touch or break any existing functionality, but I suspect in the future it may benefit from being combined with the existing view.size attribute or reimplemented in some other way.
This code extends the view metatable so that Lua code can now reference view.width and view.height to get the dimensions of any Scintilla view, or assign an integer value to view.width and view.height to set the size of any Scintilla view by adjusting the parent splits. These attributes will not resize the window and will raise an error if the resize cannot be completed due to insufficient splits.
It also adds a "resize" event that is emitted when GTK or Qt trigger a widget resize event for a Scintilla view. This can be connected to for the purpose of e.g. "pinning" a split view to a given size so that it doesn't grow when the window is resized or maximized, or for dynamically adjusting tab stops (currently unused function add_tab_stop) to center align or right align text in a buffer.
I can confirm that the changes are tested and working for Qt and curses, but I am unable to build the GTK version on MacOS again, this time due to it complaining about gtk-quartz-2.0 missing (which it isn't). I didn't want to spend time fiddling with CMake again so if someone wants to test the GTK build that would be great.
A snippet to see what the use of this is:
This is obviously less "correct" than setting a maximum size for the widget in the toolkit but it's more generalizable. You could also use it to do things like update font size based on view width. I actually think that since this implements the general case, it may be possible to eliminate the platform specific command entry handling, since the command entry view is just a Scintilla widget with a label pinned to the front. Just swap the label out for some margin text with the proper styling and it would be visually indistinguishable from what we have now, and you could probably move the focus handling and resizing over to the Lua side of the house so you wouldn't have to maintain separate logic for each platform.
Note that if you run the above snippet, there is some odd behavior (at least with Qt) where if you drag the split handle it will still move, even though the view stays the correct size. If you resize the window or any other splits it will snap back to the correct position. There is probably some update method that needs to be called at the end of set_view_dimension to notify the split to snap its handle to the correct position, but I'm not sure what that would be. It could be some kind of race condition.