quicksilver / Quicksilver

Quicksilver Project Source
http://qsapp.com
Apache License 2.0
2.73k stars 285 forks source link

Improve unsupported plugin architecture message #2872

Closed n8henrie closed 2 years ago

n8henrie commented 2 years ago

See also: https://github.com/quicksilver/Quicksilver/issues/2864

n8henrie commented 2 years ago

Old error message example:

2022-05-04 10:40:40.745568-0600 Quicksilver[11044:59603] Moving unsupported plugin 'iTunes Plugin' to PlugIns (disabled). Quicksilver only supports 64-bit plugins. i386 and PPC plugins are being disabled to avoid repeated warnings.

New example:

2022-05-10 13:58:05.584272-0600 Quicksilver[69319:20629374] Moving unsupported plugin 'E-mail Support - Private' to PlugIns (disabled). It may have an unsupported architecture.
2022-05-10 13:58:05.584355-0600 Quicksilver[69319:20629374] Detected architectures for 'E-mail Support - Private': (
    "x86_64"
)

Is this a reasonable way to go about logging this? Thanks for any feedback.

pjrobertson commented 2 years ago

Looks good!

Some small changes I'd make:

  1. Move the code that converts switch([arch intValue]) to strings to NSBundle_BLTRExtensions.m e.g. create a method called:
-(NSArray *)executableArchitecturesPretty {
    NSMutableArray *archs = [[NSMutableArray alloc] init];
        for (NSNumber *arch in [self executableArchitectures]) {
            NSString *archstr;
            switch ([arch intValue]) {
                case NSBundleExecutableArchitectureARM64:
                    archstr = @"arm64";
                    break;
                case NSBundleExecutableArchitectureI386:
                    archstr = @"i386";
                    break;
                case NSBundleExecutableArchitectureX86_64:
                    archstr = @"x86_64";
                    break;
                case NSBundleExecutableArchitecturePPC:
                    archstr = @"ppc32";
                    break;
                case NSBundleExecutableArchitecturePPC64:
                    archstr = @"ppc64";
                    break;
            }
            [archs addObject:archstr];
        }
       return archs;
}
  1. Add the message:
NSArray *archs = [[NSBundle mainBundle] executableArchitecturesPretty];
NSLog(@"Quicksilver supports the following architectures: %@", archs);
n8henrie commented 2 years ago

I'd recommend a few small changes

Sorry, saw your feedback but have been busy. Was looking at this a little yesterday -- trying to figure out No visible @interface for 'NSBundle' declares the selector 'executableArchitecturesPretty' when I've already added -(NSArray *)executableArchitecturesPretty to NSBundle_BLTRExtensions.m (inside @implementation NSBundle (BLTRExtensions))

pjrobertson commented 2 years ago

No worries, sorry - I didn't mean to be 'pushy'. Github just popped up a message saying you requested a review, and was just worried my previous messages got lost.

trying to figure out No visible @interface for 'NSBundle' declares the selector 'executableArchitecturesPretty'

You've declared it in the .m file, but if you want to make it public, you need to declare the method in the .h (header) file. Methods declared only in the .m file are only usable within that file. To let other files know about the method, declare it in the .h file:

// .h file:

-(NSArray *)executableArchitecturesPretty;

Hope that helps!

n8henrie commented 2 years ago

Oh man, I feel like a dummy. Still getting used to languages that require header files. Thanks!

n8henrie commented 2 years ago

Seems to work:

2022-05-16 11:29:09.317093-0600 Quicksilver[36427:229594] Moving unsupported plugin 'E-mail Support - Private' to PlugIns (disabled). It may have an unsupported architecture.
2022-05-16 11:29:09.317161-0600 Quicksilver[36427:229594] Detected architectures for 'E-mail Support - Private': (
    "x86_64"
)

Not sure why I had to force-push that though -- updated main and rebased prior to pushing (no conflicts). Weird.

pjrobertson commented 2 years ago

Oh man, I feel like a dummy. Still getting used to languages that require header files.

Hahaha. Header files are soooo 90s. :D

I would still also add this to the message: See my No.2 here

NSArray *archs = [[NSBundle mainBundle] executableArchitecturesPretty];
NSLog(@"Quicksilver supports the following architectures: %@", archs);
n8henrie commented 2 years ago

Sounds good.

2022-05-17 15:40:29.662987-0600 Quicksilver[80581:2789003] Moving unsupported plugin 'E-mail Support - Private' to PlugIns (disabled). It may have an unsupported architecture.
2022-05-17 15:40:29.663072-0600 Quicksilver[80581:2789003] Quicksilver supports the following architectures: (
    arm64
)
2022-05-17 15:40:29.663116-0600 Quicksilver[80581:2789003] Detected architectures for 'E-mail Support - Private': (
    "x86_64"
)
pjrobertson commented 2 years ago

👍