rogual / neovim-dot-app

Mac OS X GUI for Neovim
1.13k stars 62 forks source link

macOS 10.12 Sierra support #278

Closed mikker closed 8 years ago

mikker commented 8 years ago

Frustrated about not being able to build the app on the Sierra beta I took a look at what was needed. Turns it wasn't that much.

This will build without errors and work fine on 10.12. I am unsure about what it means for backwards compatibility though.

Thanks!

bambu commented 8 years ago

This commit does not build on Osx 10.11

Here's the relevant part:

clang++ -o build/input.o -c -std=c++11 -stdlib=libc++ -g -Wno-deprecated-register -I/usr/local/Cellar/msgpack/1.4.1/include -Ibuild -Isrc src/input.mm
src/input.mm:42:9: error: use of undeclared identifier 'NSEventTypeKeyUp'; did you mean 'NSEventTypeSwipe'?
    if (NSEventTypeKeyUp == type && NSEventModifierFlagControl & flags && 48 == [event keyCode]) {
        ^~~~~~~~~~~~~~~~
        NSEventTypeSwipe
/System/Library/Frameworks/AppKit.framework/Headers/NSEvent.h:49:5: note: 'NSEventTypeSwipe' declared here
    NSEventTypeSwipe   NS_ENUM_AVAILABLE_MAC(10_5)       = 31,
    ^
src/input.mm:42:37: error: use of undeclared identifier 'NSEventModifierFlagControl'
    if (NSEventTypeKeyUp == type && NSEventModifierFlagControl & flags && 48 == [event keyCode]) {
                                    ^
2 errors generated.
scons: *** [build/input.o] Error 1
make: *** [all] Error 2

It seems that NSEventTypeKeyUp and NSEventModifierFlagControl are both added in macOS 10.12

mikker commented 8 years ago

Maybe this will work, @bambu ?

bambu commented 8 years ago

That still didn't build. very similar error if not the same. How about this:

diff --git a/src/input.mm b/src/input.mm
index 3b63320..7eba3b5 100644
--- a/src/input.mm
+++ b/src/input.mm
@@ -39,21 +39,15 @@

     /* <C-Tab> & <C-S-Tab> do not trigger keyDown events.
        Catch the key event here and pass it to keyDown. */
-    if ([[NSProcessInfo processInfo] operatingSystemVersion].minorVersion <= 11) {
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-      if (NSKeyDown == type && NSControlKeyMask & flags && 48 == [event keyCode]) {
-          [self keyDown:event];
-          return YES;
-      }
-#pragma clang diagnostic pop
-    } else {
-      if (NSEventTypeKeyDown == type && NSEventModifierFlagControl & flags && 48 == [event keyCode]) {
-          [self keyDown:event];
-          return YES;
-      }
-    }
-   
+#ifdef __MAC_10_12
+  if (NSEventTypeKeyDown == type && NSEventModifierFlagControl & flags && 48 == [event keyCode]) {
+#else
+  if (NSKeyDown == type && NSControlKeyMask & flags && 48 == [event keyCode]) {
+#endif
+      [self keyDown:event];
+      return YES;
+  }
+
     return NO;
 }
mikker commented 8 years ago

I thought I had searched everywhere for a macro like that 😀 Thanks! Works flawlessly with your patch.

bambu commented 8 years ago

No worries. I only know it because i've used it before. You want to modify your PR or should I make another?

mikker commented 8 years ago

If this is fine for you then it's also fine for me.

james2doyle commented 8 years ago

I actually can't run the latest version after this commit. The app will build fine, but will hang and never start the window. I'm on 10.11.5

bambu commented 8 years ago

@james2doyle, that seems odd. This code change shouldn't cause anything like that. I too am on 10.11.5.

Here are a few things to try:

  1. Are you able to revert to the previous commit, build and run just fine?
  2. By "hang and never start the window" do you mean that you get a grey window with nothing on it?
  3. If you press any keys (enter for example) does it pop up?
  4. If you just run nvim from command line do you get any messages?

Finally, what version of Neovim are you running?

james2doyle commented 8 years ago

The app shows up in the dock, but a window never appears and the app never focuses. I have to force close it after about a minute.

Maybe I should mention I'm installing via brew. I used brew install --HEAD neovim-dot-app.

I upgraded neovim before trying to update the app. Also, no messages running neovim from the command line.

On Thu, Jul 14, 2016, 7:36 PM Bambu notifications@github.com wrote:

@james2doyle https://github.com/james2doyle, that seems odd. This code change shouldn't cause anything like that. I too am on 10.11.5.

Here are a few things to try:

  1. Are you able to revert to the previous commit, build and run just fine?
  2. By "hang and never start the window" do you mean that you get a grey window with nothing on it?
  3. If you press any keys (enter for example) does it pop up?
  4. If you just run nvim from command line do you get any messages?

Finally, what version of Neovim are you running?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/rogual/neovim-dot-app/pull/278#issuecomment-232845793, or mute the thread https://github.com/notifications/unsubscribe-auth/ABW_mB6zrfhbMKgYVk2q_y8K2VkvDPVJks5qVvIUgaJpZM4JI6mg .

bambu commented 8 years ago

@james2doyle how about now?

james2doyle commented 8 years ago

😖

Still busted. Here is my Console.app while running:

screen shot 2016-07-14 at 8 05 47 pm

Not sure if that helps

james2doyle commented 8 years ago

Hey man. Really sorry about this but I restarted on a whim, and now it works. I didn't turn it on and off again. Again, sorry!

rogual commented 8 years ago

OSX does maintain backwards compatibility — correct solution would be to use -mmacosx-version-min compile flag (or possibly one of the defines; can't remember right now) and continue using the old constants.

Edit: The macros are detailed here:

             This header enables a developer to specify build time
             constraints on what Mac OS X versions the resulting
             application will be run.  There are two bounds a developer
             can specify:

                  MAC_OS_X_VERSION_MIN_REQUIRED
                  MAC_OS_X_VERSION_MAX_ALLOWED

            The lower bound controls which calls to OS functions will 
            be weak-importing (allowed to be unresolved at launch time).
            The upper bound controls which OS functionality, if used,
            will result in a compiler error because that functionality is
            not available on on any OS is the specifed range.
bambu commented 8 years ago

@rogual is correct. I had forgotten about that flag. I originally suggested __MAC_10_12 because i remembered it being in use in the current code base for a different version. Looking at it shows that it had its specific use case. Since the minimum requirements for Neovim.app (according to README) is 10.9, having -mmacosx-version-min=10.9 in the SConstruct is the better answer so that we don't run into these issues any more.