-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Add flex resize, focus mode, hard-coded limits for now (clone of #2704) #8546
base: master
Are you sure you want to change the base?
Conversation
If you're re-submitting someone else's code it's customary to credit them as a co-author in the commits: https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors |
1238d49
to
3b59425
Compare
If it's as far as simply rebasing the original work I'd keep the commit authorship as is and either add a Co-authored-by or a separate commit with the fixes |
277910a
to
165070b
Compare
2bed1e6
to
982c008
Compare
982c008
to
1d79ba9
Compare
There was a problem with author's name on old commits. Now everything looks fine |
This comment was marked as spam.
This comment was marked as spam.
332cc8e
to
7c54c63
Compare
Is there anything I can do to help get this PR merged? Would be great to have this feature. |
0dde7a6
to
7bb4ed2
Compare
"A-w" => { "Alter Window" | ||
"A-h"|"A-left" |"h"|"left" => shrink_buffer_width, | ||
"A-l"|"A-right"|"l"|"right" => grow_buffer_width, | ||
"A-j"|"A-down" |"j"|"down" => shrink_buffer_height, | ||
"A-k"|"A-up" |"k"|"up" => grow_buffer_height, | ||
"A-f"|"f" => toggle_focus_window, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we need the non-sticky version of this at all? I don't imagine that you would want to make a single edit to the sizes.
I'm also not sure about the keybinding. Instead I think this might fit better as a minor mode under C-w
/<space>w
specifically for resizing and move the f
binding under C-w
/<space>w
instead
pub fn resize_buffer(&mut self, resize_type: Resize, dimension: Dimension) { | ||
self.tree.resize_buffer(resize_type, dimension); | ||
} | ||
|
||
pub fn toggle_focus_window(&mut self) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's some inconsistency here between "buffer" and "window". I would call both of these "view"s to match the internal wording
fn grow_buffer_width(cx: &mut Context) { | ||
cx.editor.resize_buffer(Resize::Grow, Dimension::Width); | ||
} | ||
|
||
fn shrink_buffer_width(cx: &mut Context) { | ||
cx.editor.resize_buffer(Resize::Shrink, Dimension::Width); | ||
} | ||
fn grow_buffer_height(cx: &mut Context) { | ||
cx.editor.resize_buffer(Resize::Grow, Dimension::Height); | ||
} | ||
|
||
fn shrink_buffer_height(cx: &mut Context) { | ||
cx.editor.resize_buffer(Resize::Shrink, Dimension::Height); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This strikes me as not the right API for resizing. I would expect that commands would hardcode the amount/ratio or take the cx.count()
into account to decide how much to resize. With this API it's hard to introduce resizing windows by mouse dragging the separator for example, and we can take the terminals cells into account to give an exact amount. I think @pascalkuthe has similar thoughts here if he'd like to weigh in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For an example of another pane resizing thing in the wild: in sway/i3 you can create a mode to resize like I have here: https://github.com/the-mikedavis/dotfiles/blob/51d85cb1f258300b6fc90427fe7c1069ff924a64/defaults/sway/default.nix#L255-L271
That's what I'm thinking for the sticky mode and the hardcoding of amounts to resize by
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I think using terminal cells/pixels as aboslute positions instead of these relative slots makes more sense to me
This sure would be a great feature if we could just get that second reviewer so this can be merged that would be great! Please and thank you! |
This feature had been implemented in #2704, but PR seams inactive. I really wanted this feature and just copy pasted code from original PR.
Fixes #1176