Skip to content

Support guioptions 'k' flag in MacVim, prevents unnecessary window re… #708

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

Merged
merged 1 commit into from
Jul 9, 2018

Conversation

ychin
Copy link
Member

@ychin ychin commented Jul 8, 2018

…size

Adds support for native GVim's 'k' flag. Adding/removing tabs/scrollbars
and setting 'linespace'/'columnspace' would now cause the number of
lines and columns in the buffer change to keep the window size constant,
instead of the other way round of resizing the window to keep the view
size constant. This helps prevent the window from resizing itself
unncessarily, which could be especially annoying when the window is
pinned/maximized.

Manually setting 'lines'/'columns', going to full screen, dragging the
window corner to resize would still resize the window.

Also removed misc calls within MMWindowController.m that were setting
shouldResizeVimView. Those calls were already handled by native Vim's
gui.c's gui_set_shellsize so it's redundant.

Close #617

…size

Adds support for native GVim's 'k' flag. Adding/removing tabs/scrollbars
and setting 'linespace'/'columnspace' would now cause the number of
lines and columns in the buffer change to keep the window size constant,
instead of the other way round of resizing the window to keep the view
size constant. This helps prevent the window from resizing itself
unncessarily, which could be especially annoying when the window is
pinned/maximized.

Manually setting 'lines'/'columns', going to full screen, dragging the
window corner to resize would still resize the window.

Also removed misc calls within MMWindowController.m that were setting
shouldResizeVimView. Those calls were already handled by native Vim's
gui.c's gui_set_shellsize so it's redundant.

Close macvim-dev#617
@ychin
Copy link
Member Author

ychin commented Jul 8, 2018

I couldn't quite find any unit tests specific to MacVim in the repo (raw Vim has the src/testdir which has a collection of Vim scripts for unit tests). If I missed that I can add unit tests for this functionality as well.

@splhack splhack merged commit 378032d into macvim-dev:master Jul 9, 2018
@ychin ychin deleted the guioptions-k branch July 9, 2018 06:28
splhack added a commit that referenced this pull request Aug 3, 2018
Binary targets macOS 10.8+

- Vim patch 8.1.0235
- Touch Bar support #715
- Force click support #716
- New guioption 'k' #708
- Fix CoreText renderer

Script interfaces have compatibility with these versions

- Lua 5.3
- Perl 5.18
- Python2 2.7
- Python3 3.7
- Ruby 2.5
@ichizok ichizok mentioned this pull request Aug 8, 2018
@ichizok
Copy link
Member

ichizok commented Aug 10, 2018

@splhack @ychin
It appears this change has caused #721, so I think should revert it once.
BTW, this guioptions-k does not work in my environment (using d0807a4).

@ychin
Copy link
Member Author

ychin commented Aug 10, 2018

Ok. Feel free to revert the change. I'll try to look into the issue and submit a proper fix. The way the messages got bounced when resizing windows involved quite a few roundtrips, and it's possible I missed some cases since I didn't test on another monitor.

@ychin
Copy link
Member Author

ychin commented Aug 10, 2018

Also @ichizok how does guioptions-k not work for you? Can you give a more detailed set of repro steps? I'm curious as it could be related why it's causing issues.

@ychin
Copy link
Member Author

ychin commented Aug 10, 2018

@ichizok @splhack Opened new pull request (#727) to revert this change. Sorry for the breakage.

@ichizok
Copy link
Member

ichizok commented Aug 10, 2018

Repro steps

Start MacVim with GUI:
/path/to/MacVim.app/Contents/MacOS/Vim --clean -g

Set guioptions-k and then guioptions-T (add toolbar):

:set go+=k
:set go+=T

Expected: Shows toolbar, and window size (height) does not change.
Actual: Shows toolbar, and window height increases by toolbar height.

MacVim:

guioptions-k-mac

My understanding is that the following GVim (linux) is the correct behavior.
With guioptions-k, the window height does not change by showing/hiding toolbar.

GVim (on Ubuntu):

guioptions-k-linux

@ychin
Copy link
Member Author

ychin commented Aug 10, 2018

Ah yes, right. I think there's actually a bug in that toolbars don't work, but scrollbars / linespace / tabs do work. I will fix that case as well in the second version of this pull request later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants