-
-
Notifications
You must be signed in to change notification settings - Fork 685
Support guioptions 'k' flag in MacVim (2nd attempt) #731
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
Conversation
…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. |
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
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
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!