Skip to content
This repository has been archived by the owner on Sep 4, 2018. It is now read-only.

Added correct detection of installed ideplugins #369

Merged
merged 2 commits into from Nov 27, 2015
Merged

Added correct detection of installed ideplugins #369

merged 2 commits into from Nov 27, 2015

Conversation

ksuther
Copy link
Contributor

@ksuther ksuther commented Nov 1, 2015

Xcode has multiple types of plugin prefixes. Most plugins are xcplugins and can be installed in ~/Library/Application Support/Developer/Shared/Xcode/Plug-ins, but ideplugins have to be installed in ~/Library/Developer/Xcode/Plug-ins. KSImageNamed switched from being an xcplugin to an ideplugin, so Alcatraz no longer shows it as installed.

pathForInstalledPackage: will now detect if it's dealing with an ideplugin that is installed and change the path to the correct one.

Added a \\s to the end of the regex to weed out false positives (such as MyPlugin.xcplugindata). The extra whitespace in the result gets trimmed afterwards.
pathForInstalledPackage: now returns ~/Library/Developer/Xcode/Plug-ins for ideplugins, as they're not loaded if they're placed in ~/Library/Application Support/Developer/Shared/Xcode/Plug-ins.
stringByAppendingString:package.extension];
// Packages can be installed in multiple places:
// ~/Library/Application Support/Developer/Shared/Xcode/Plug-ins (where most xcplugins go)
// ~/Library/Developer/Xcode/Plug-ins (where ideplugins go)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Avoid adding comments in the code detailing the changes, use Github issues instead

It could make sense to pull this out into a predicate method or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't follow what you mean by a predicate method in this case. Do you mean adding something like -(NSString*)installPath to ATZPlugin? I could do that, although that would mean stuff like installNameFromPbxproj would start leaking out of ATZPluginInstaller.

Should I remove those comments? I don't see how that's a comment detailing a change, it's saying what the code is actually doing. I'll remove it if that's the requested style.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nope, that's not what I meant, I meant the pathname check.

I think the code is mostly clear enough and we prefer self explanatory code over comments :)

@jurre
Copy link
Collaborator

jurre commented Nov 1, 2015

Thanks for the PR!

I've left an inline comment with a small nitpick

@joliylee
Copy link

joliylee commented Nov 4, 2015

Thanks for the PR!

kattrali added a commit that referenced this pull request Nov 27, 2015
Added correct detection of installed ideplugins
@kattrali kattrali merged commit c07e386 into alcatraz:master Nov 27, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants