Support guioptions 'k' flag in MacVim (2nd attempt) #731
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…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. rebased from 1333bc6
Fix the following: * Zoom button: There was an error in refactoring leading to the handler calling the wrong function. * Toolbar addition/removal: This now respects the 'k' option. Toolbar is different from scrollbar and tabs because when you add/remove a toolbar in Cocoa it automatically resizes the window for you, so the implementation needs to manually un-resize the window and re-calculates the Vim view's 'lines' and 'columns' to fit.
|
Unfortunately, it looks like the problem persists with this build. |
|
Ok thanks for testing it out. I’ll find an external monitor to test it out. |
|
I believe I have fixed all the issues introduced in the original implementation. Those are:
Pinging @dctucker @ichizok again. Also @splhack if he wants to merge it. I tested this in an external monitor after the fix (yay USB-C dongle!) and couldn't reproduce it. I'm pretty confident the issue is now fixed. |
Fix the issue that MacVim's window will have broken rendering (wrong Vim size) if the window was opened in another monitor. This was introduced as part of the implementation for guioptions 'k', The issue was that the function `moveWindowAcrossScreens` was buggy. It sets a flag "resizingDueToMove" but doesn't unset it after the `setFrameTopLeftPoint`, which may or may not call the resize function that is responsible in unsetting "resizeDueToMove". Fix the function to always unset it so that the flag doesn't leak till the next resize. Previously it "worked" due to MacVim's excessive resize messages masking the issue.
|
@ychin Thank you, I confirmed the fix about toolbar on/off with guioptions-k. |
|
Is there anything else I need to do for this pull request? I am pretty sure I addressed the issues caused by the original change and I have been using the editor after my fix and they seem to work fine. I have a few additional fixes for some full screen rendering and resizing bugs but they rely on this change so wondering what I can do to get this merged. Thanks. |
ychin
added a commit
to ychin/macvim
that referenced
this pull request
Dec 5, 2018
Vim patch 8.1.560
Targets macOS 10.8+
Features:
- macOS Mojave (10.14) is now supported.
- MacVim's UI now works with Dark Mode.
- Fixed broken rendering and flickering under Mojave when using the
default Core Text renderer. macvim-dev#757
- guioption 'k' is supported again. macvim-dev#731
- This option prevents window from resizing when UI elements such as
toolbars or tabs show or hide themselves.
Fixes:
- Fixed misc fullscreen and window resizing bugs and artifacts macvim-dev#745
- Dragging tabs to reorder now works properly macvim-dev#789
- Fixed timer callback handling in GUI macvim-dev#749
- Fixed native tabs (10.12+) interring with Vim tabs macvim-dev#788
- Fixed Japanese IME Ctrl-U/Ctrl-O handling macvim-dev#742
- Fixed MMShareFindPboard and Cmd-E/Cmd-G interactions macvim-dev#780
- Better handling of guifontwide font size macvim-dev#737
- Better python discovery in default vimrc macvim-dev#739
Known Issues:
- Scrolling performance is slightly worse under Mojave macvim-dev#796
Script interfaces have compatibility with these versions:
- Lua 5.3
- Perl 5.18
- Python2 2.7
- Python3 3.7
- Ruby 2.5
ychin
added a commit
that referenced
this pull request
Dec 5, 2018
Vim patch 8.1.560
Targets macOS 10.8+
Features:
- macOS Mojave (10.14) is now supported.
- MacVim's UI now works with Dark Mode.
- Fixed broken rendering and flickering under Mojave when using the
default Core Text renderer. #757
- guioption 'k' is supported again. #731
- This option prevents window from resizing when UI elements such as
toolbars or tabs show or hide themselves.
Fixes:
- Fixed misc fullscreen and window resizing bugs and artifacts #745
- Dragging tabs to reorder now works properly #789
- Fixed timer callback handling in GUI #749
- Fixed native tabs (10.12+) interring with Vim tabs #788
- Fixed Japanese IME Ctrl-U/Ctrl-O handling #742
- Fixed MMShareFindPboard and Cmd-E/Cmd-G interactions #780
- Better handling of guifontwide font size #737
- Better python discovery in default vimrc #739
Known Issues:
- Scrolling performance is slightly worse under Mojave #796
Script interfaces have compatibility with these versions:
- Lua 5.3
- Perl 5.18
- Python2 2.7
- Python3 3.7
- Ruby 2.5
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This re-commits and fixes the issues introduced in the previous pull request that implemented guioptions 'k' (#708). That commit was reverted in #727.
The issues it fixes:
Zoom button ("zoom" option in window menu stopped working with snapshot-150 #724): There was an error in refactoring leading to the handler calling the wrong function.
Toolbar addition/removal: Toolbar now respects the 'k' option. Toolbar is different from scrollbar and tabs because when you add/remove a toolbar in Cocoa it automatically resizes the window for you, so the implementation needs to manually un-resize the window and re-calculates the Vim view's 'lines' and 'columns' to fit.
NOTE: There's another issue where the last implementation broke MacVim on external monitors (#721 and #728). I do not have access to an external monitor now and can't test whether this fixes it (no USB-C adapter). I looked through the code and still couldn't figure out why the external monitor would cause issues especially if the user hasn't set
go+=k. I will need to find an external monitor to test, but if someone can quickly try this out and sanity check it I would appreciate it.Please do not merge yet until I can verify that the external monitor issue is fixed.
Edit: The external monitor issue is now fixed (e35b0a6). This PR should be OK to merge!