-
Notifications
You must be signed in to change notification settings - Fork 823
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
Feature: Ability to set executable to Ask to Launch #871
Conversation
For Share Extensions, is it valid for the scheme to be setup either way? If not, should instead default this based on the target type? |
It is valid to be set up both ways. If you want to run the share extension directly you had to manually change it to Ask to Launch, but its not a requirement to do so. IMO it would be better to make it the default but this way you have the ability without forcing others to default to it. |
What does Xcode default to when creating a similar scheme? |
Defaults to the settings that I created, I can add it as a default as well. Can you point me in the direction where I can find the defaults? |
@@ -241,6 +241,7 @@ public class SchemeGenerator { | |||
macroExpansion: shouldExecuteOnLaunch ? nil : buildableReference, | |||
selectedDebuggerIdentifier: (scheme.run?.debugEnabled ?? Scheme.Run.debugEnabledDefault) ? XCScheme.defaultDebugger : "", | |||
selectedLauncherIdentifier: (scheme.run?.debugEnabled ?? Scheme.Run.debugEnabledDefault) ? XCScheme.defaultLauncher : "Xcode.IDEFoundation.Launcher.PosixSpawn", | |||
askForAppToLaunch: scheme.run?.askForAppToLaunch, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You would default here, using information from the rest of the target. If you have trouble with that, we can leave the existing behavior as the default for now.
@brentleyjones when one of the build targets is an app extension it will default to these settings, but still have the ability to override them in the yaml as people see fit. |
@@ -364,6 +367,10 @@ extension Scheme.Run: JSONObjectConvertible { | |||
} else if let string: String = jsonDictionary.json(atKeyPath: "launchAutomaticallySubstyle") { | |||
launchAutomaticallySubstyle = string | |||
} | |||
|
|||
if let askLaunch: Bool = jsonDictionary.json(atKeyPath: "askForAppToLaunch") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about defaulting to false
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the key is by default non-existing instead of false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this value were false, then the property would be nil. Is that what is intended?
@@ -173,7 +173,7 @@ public class SchemeGenerator { | |||
|
|||
let buildableReference = buildActionEntries.first(where: { $0.buildableReference.blueprintName == schemeTarget?.name })?.buildableReference ?? buildActionEntries.first!.buildableReference | |||
let runnables = makeProductRunnables(for: schemeTarget, buildableReference: buildableReference) | |||
|
|||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally find it worth it to remove dead whitespace for cleaner diffs.
eb0542b
to
7120a91
Compare
@@ -233,6 +233,14 @@ public class SchemeGenerator { | |||
} | |||
locationScenarioReference = XCScheme.LocationScenarioReference(identifier: identifier, referenceType: referenceType.rawValue) | |||
} | |||
|
|||
let appExtensionTarget = scheme.build.targets.compactMap { bt -> Target? in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we instead just use schemeTarget
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in my case the share extension needs both the normal app and the extension set for build targets. When either one of the targets is the app extension I set it to askForAppToLaunch otherwise the key can be nil. Using the schemeTarget doesn't have the same result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, okay. Let's not default it at all then. If its not specified in the yml
we will behave like we do now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me, this way we don't enforce anything upon the user but let them take care of setting the right values in the yml
.
@pinda This looks good. Can you add a changelog entry and a simple unit test as well? Thanks! |
Thanks again @pinda! |
Thanks for this @pinda. Could you please make a followup PR with an update to the project spec https://github.com/yonaskolb/XcodeGen/blob/master/Docs/ProjectSpec.md @brentleyjones, it's something we should require when new properties are added 👍 |
For Share Extensions it allows the possibility to set the executable to Ask to Launch.