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

Improve unsupported plugin architecture message #2872

Merged
merged 3 commits into from
May 18, 2022
Merged

Conversation

n8henrie
Copy link
Member

See also: #2864

@n8henrie n8henrie marked this pull request as draft May 10, 2022 20:00
@n8henrie n8henrie marked this pull request as ready for review May 10, 2022 20:00
@n8henrie
Copy link
Member Author

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

pjrobertson commented May 10, 2022

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);

Copy link
Member

@pjrobertson pjrobertson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comments in the PR main thread. I'd recommend a few small changes

@n8henrie
Copy link
Member Author

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

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

n8henrie commented May 16, 2022 via email

@n8henrie
Copy link
Member Author

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

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

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

👍

@pjrobertson pjrobertson merged commit eec6e5f into main May 18, 2022
@pjrobertson pjrobertson deleted the issue_2864 branch May 18, 2022 14:43
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.

None yet

2 participants