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

macOS 10.12 Sierra support #278

Merged
merged 2 commits into from
Jul 15, 2016
Merged

macOS 10.12 Sierra support #278

merged 2 commits into from
Jul 15, 2016

Conversation

mikker
Copy link
Contributor

@mikker mikker commented Jul 10, 2016

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
Copy link
Contributor

bambu commented Jul 11, 2016

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
Copy link
Contributor Author

mikker commented Jul 12, 2016

Maybe this will work, @bambu ?

@bambu
Copy link
Contributor

bambu commented Jul 13, 2016

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
Copy link
Contributor Author

mikker commented Jul 13, 2016

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

@bambu
Copy link
Contributor

bambu commented Jul 14, 2016

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

@mikker
Copy link
Contributor Author

mikker commented Jul 14, 2016

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

@bambu bambu merged commit 587b9fb into rogual:master Jul 15, 2016
@james2doyle
Copy link
Contributor

james2doyle commented Jul 15, 2016

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
Copy link
Contributor

bambu commented Jul 15, 2016

@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
Copy link
Contributor

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
#278 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABW_mB6zrfhbMKgYVk2q_y8K2VkvDPVJks5qVvIUgaJpZM4JI6mg
.

@bambu
Copy link
Contributor

bambu commented Jul 15, 2016

@james2doyle how about now?

@james2doyle
Copy link
Contributor

😖

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
Copy link
Contributor

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
Copy link
Owner

rogual commented Jul 15, 2016

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
Copy link
Contributor

bambu commented Jul 15, 2016

@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.

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.

4 participants