Skip to content
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

Fix misc MacVim warnings #1303

Merged
merged 8 commits into from
Oct 6, 2022
Merged

Fix misc MacVim warnings #1303

merged 8 commits into from
Oct 6, 2022

Conversation

ychin
Copy link
Member

@ychin ychin commented Oct 6, 2022

MacVim currently has a lot of warnings, especially when compiling locally with a higher deployment target than 10.9 (which the CI uses). This PR fixes most of them to help reduce on the spam on the road to being able to turn on warnings as errors. Most of the warnings are deprecation warnings, and since we still support 10.9, the work needs to make sure to maintain compatibility with older versions of macOS and also code compatibility. This PR would also help make the build output be a lot less spammy once we move the main binary to build against 10.13 once Xcode 14.1 comes out and links against macOS 13.0 SDK (which only supports down to 10.13) instead of 12.0 SDK that we use now (which supports down to 10.9).

The PR is broken into a lot of smaller commits to make the work easier to follow and easier to revert if something goes wrong.

After this PR, there are a few items left:

  • NSConnection / etc warnings. This is the big one but fixing it requires completely changing our IPC mechanism. It will be handled by Investigate moving away from DO (Distributed Object) as IPC mechanism #1157.
  • NSToolbarSeparatorItemIdentifier. These are used for the -sep- tool bar items and apparently they haven't been working for a loong time. They will be addressed in a future PR which will introduce toolbar/menu/touch bar changes like SF symbol support.
  • Misc setTarget/setAction warnings when using PSMTabBar. These are minor, but since we plan to replace our tab implementation in Tabs #1120 anyway there's no need to fix them and add merge conflicts.
  • Random .xib warnings about sizing constraints etc. I have never seen this to become an actual issue, and this won't stop us from turning on warnings as errors (since these aren't source code) and so not fixing them for now.

commit c41e05b

Fix warning: zoomAll / makeWindowsPerform

Fix the deprecation warning on makeWindowsPerform. While we could simply replace it with the more updated form, one thing I noticed was that zoomAll: simply wasn't getting called. It appears that NSApplication has a native handler of it and would call zoom: on each window by itself, and as such there's no point in making our own zoomAll: method at all. I couldn't find out if this was the case in old macOS versions, and so just ifdef out the zoomAll function in newer versions of macOS which also fixes the deprecation warning.


commit dc1ad36

Fix NSFilenamesPboardType -> NSPasteboardTypeFileURL deprecation warning

This is a little tricky because it's not a simple map. With NSPasteboardTypeFileURL, we have to use readObjectsForClasses:options: to obtain a list of URL objects and then turn them into file path strings. Previously you could just get a list of file names directly with NSFilenamesPboardType. Also, this new enum was only defined in 10.13, so we have to maintain parallel paths to support both types of getting the list of file names from the pasteboard. Also refactored the code that use this to a function in Miscllaneous.m to avoid the headache. Note that the old NSFilenamesPboardType method still works today, so this is mostly to clean up old code and deprecation warnings.

Also made the "new file here" plugin accept both the old and new pasteboard types, just in case.


commit 94564fd

Fix NSWindowStyleMaskTexturedBackground deprecation warning

Seems like the flag has been completely useless since macOS 11. We previously kept it around despite its deprecation status because it seems to make the title bar not show a black line, but since macOS 11 it has been showing that anyway, so remove usage of it. Also, clean up misc pre-Lion-era code and block them behind ifdef. Can remove those code in future if we want to clean up.


commit 2faa285

Fix MMTouchBarButton warning when calling "desc" function

Refactor the code so that the relevant class is in header and can be called. Also fix different places where I call NSClassFromString to use @available check instead, as I believe that's the recommended method and more efficient as well (due to it being native to the compiler).


commit cd32577

Fix MacVim warnings: enum renames, graphicsPort, setcmdheight

The list of warnings fixed:

  • Fix misc AppKit control states enums that got renamed and deprecated.
  • NSFindPboardType -> NSPasteboardTypeFind deprecation.
  • Fix usage of deprecated "graphicsPort" API to use CGContext instead.
  • Use NSFontChanging protocol if it's available.
  • Move MMCoreTextView's setcmdheight to the correct section in the private implementation category.

commit 2e67bf4

Fix warnings due to usages of the deprecated NSStringPboardType

This turned out more complicated than I thoguht because the newer NSPasteboardTypeString (public.utf8-plain-text) is actually a different value from NSStringPboardType (NSStringPboardType), so it could potentially lead to behavior differences. The right-click Services menu in particular seems to not behave in the expected way, because writeSelectionToPasteboard: (called by the OS) is passing NSStringPboardType to us, even though we specifically only accept NSPasteboardTypeString in validRequestorForSendType:returnType:. Just fixed the code to ignore the passed in type.

Also update the Info.plist file to accept the public.utf8-plain-text for this service as well.


commit 2a6dea1

Fix build phase dependency warnings

Make sure to specify the build phase (e.g. building locale files) input/output dependencies so they can be properly skipped during incremental builds if the input files haven't changed. Previously some of them were set to use dependency tracking, but with no input/output specified, therefore triggering a warning as Xcode had to run them every build.


commit e9e1290

Fix MacVim compiler warnings (for Vim-process files)

Fix NSStringPboardType and NSFindPboard deprecation warnings.

Also, fix up an awkward use of colors in that we are loading the ARGB values of system colors using device calibrated space but CoreText renderer uses sRGB instead. Just load it as sRGB.

This should fix up all Vim-side compiler warnings except for the usage of NSConnection, which is a much larger task to tackle as we need to move to XPC. Note that the set of warnings differ depending on whether we have MACOSX_DEPLOYMENT_TARGET=10.9 set or not as Xcode will recommend different changes. With that set we currently do not throw any warnings on the Vim process side (since NSConnection was not deprecated at 10.9 yet).

ychin added 8 commits October 6, 2022 01:04
Fix NSStringPboardType and NSFindPboard deprecation warnings.

Also, fix up an awkward use of colors in that we are loading the ARGB
values of system colors using device calibrated space but CoreText
renderer uses sRGB instead. Just load it as sRGB.

This should fix up all Vim-side compiler warnings except for the usage
of NSConnection, which is a much larger task to tackle as we need to
move to XPC. Note that the set of warnings differ depending on whether
we have `MACOSX_DEPLOYMENT_TARGET=10.9` set or not as Xcode will
recommend different changes. With that set we currently do not throw any
warnings on the Vim process side (since NSConnection was not deprecated
at 10.9 yet).
Make sure to specify the build phase (e.g. building locale files)
input/output dependencies so they can be properly skipped during
incremental builds if the input files haven't changed. Previously some
of them were set to use dependency tracking, but with no input/output
specified, therefore triggering a warning as Xcode had to run them every
build.
This turned out more complicated than I thoguht because
the newer NSPasteboardTypeString (public.utf8-plain-text) is actually a
different value from NSStringPboardType (NSStringPboardType), so it
could potentially lead to behavior differences. The right-click Services
menu in particular seems to not behave in the expected way, because
writeSelectionToPasteboard: (called by the OS) is passing
NSStringPboardType to us, even though we specifically only accept
NSPasteboardTypeString in validRequestorForSendType:returnType:. Just
fixed the code to ignore the passed in type.

Also update the Info.plist file to accept the public.utf8-plain-text for
this service as well.
The list of warnings fixed:

- Fix misc AppKit control states enums that got renamed and deprecated.
- NSFindPboardType -> NSPasteboardTypeFind deprecation.
- Fix usage of deprecated "graphicsPort" API to use CGContext instead.
- Use NSFontChanging protocol if it's available.
- Move MMCoreTextView's setcmdheight to the correct section in the private
  implementation category.
Refactor the code so that the relevant class is in header and can be
called. Also fix different places where I call NSClassFromString to use
@available check instead, as I believe that's the recommended method and
more efficient as well (due to it being native to the compiler).
Seems like the flag has been completely useless since macOS 11. We
previously kept it around despite its deprecation status because it
seems to make the title bar not show a black line, but since macOS 11 it
has been showing that anyway, so remove usage of it. Also, clean up misc
pre-Lion-era code and block them behind ifdef. Can remove those code in
future if we want to clean up.
This is a little tricky because it's not a simple map. With
NSPasteboardTypeFileURL, we have to use readObjectsForClasses:options:
to obtain a list of URL objects and then turn them into file path
strings. Previously you could just get a list of file names directly
with NSFilenamesPboardType. Also, this new enum was only defined in
10.13, so we have to maintain parallel paths to support both types of
getting the list of file names from the pasteboard. Also refactored the
code that use this to a function in Miscllaneous.m to avoid the
headache. Note that the old NSFilenamesPboardType method still works
today, so this is mostly to clean up old code and deprecation warnings.

Also made the "new file here" plugin accept both the old and new
pasteboard types, just in case.
Fix the deprecation warning on makeWindowsPerform. While we could simply
replace it with the more updated form, one thing I noticed was that
zoomAll: simply wasn't getting called. It appears that NSApplication has
a native handler of it and would call zoom: on each window by itself,
and as such there's no point in making our own zoomAll: method at all. I
couldn't find out if this was the case in old macOS versions, and so
just ifdef out the zoomAll function in newer versions of macOS which
also fixes the deprecation warning.
@ychin ychin added this to the Release 175 milestone Oct 6, 2022
@ychin ychin merged commit f6cabb1 into macvim-dev:master Oct 6, 2022
@ychin ychin deleted the warnings-fixes branch October 6, 2022 22:16
@ychin ychin added the Non User Facing Non-user facing change. These issues do no need to show up in release notes. label Oct 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Non User Facing Non-user facing change. These issues do no need to show up in release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant