Skip to content

Add inline #question syntax for import questions#154

Open
maximilianspitzer wants to merge 5 commits into
electrikmilk:mainfrom
maximilianspitzer:feature/inline-import-questions
Open

Add inline #question syntax for import questions#154
maximilianspitzer wants to merge 5 commits into
electrikmilk:mainfrom
maximilianspitzer:feature/inline-import-questions

Conversation

@maximilianspitzer

Copy link
Copy Markdown
Contributor

Summary

Adds support for declaring import questions directly inside action arguments using inline #question "prompt" syntax. This is useful for third-party App Intent entity parameters (Notion databases, Slack channels, WhatsApp contacts, etc.) where Shortcuts shows the app's native picker at install time.

Before — only variable-style questions with required default:

#question db "Pick a database" "default"
createNotionPage(db, "Title", nil, "Body")

After — inline syntax, default value optional:

createNotionPage(#question "Which database?", "Title", nil, "Body")

The compiler resolves the correct ActionIndex and ParameterKey automatically since the question is positionally bound to the parameter. The legacy #question variable syntax continues to work, and default values are now optional for both forms.

Also fixes third-party action identifiers like notion.id.CreatePageWithoutOpenIntent being incorrectly prefixed with is.workflow.actions. (dot-count threshold changed from < 4 to < 3).

Changes

  • token.go — Add InlineQuestion value type and inlineQuestionValue struct
  • parser.go — Parse inline #question "prompt" in collectValue(), make default value optional for legacy #question
  • action.go — Collect inline questions in makeAction() with correct action index from generated output; skip in makeActionParams(); fix identifier dot threshold
  • shortcutgen.go — Move generateImportQuestions() after generateActions(); merge legacy and inline questions; add omitempty on DefaultValue; reset inlineQuestions in resetShortcutGen()
  • cherri_test.go — Reset inlineQuestions in resetParser()

Testing

  • All 33 existing tests pass
  • Compiled a Notion shortcut using #question for the database parameter — produced correct plist with ActionIndex: 6, ParameterKey: "database", no DefaultValue
  • Installed and verified the Shortcuts import question triggers Notion's database picker

Copilot AI review requested due to automatic review settings March 19, 2026 19:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds support for inline #question "prompt" import-question arguments (with optional default) so App Intent entity parameters can trigger native pickers at install time, while preserving legacy #question var syntax. It also adjusts third-party action identifier handling to avoid incorrectly prefixing identifiers that already include a bundle-style namespace.

Changes:

  • Parse inline #question "prompt" (and optional "default") as an argument value type and collect those questions during generation.
  • Make legacy #question default values optional and omit DefaultValue from plist when absent.
  • Fix action identifier dot-threshold logic for deciding when to treat a quoted identifier as a full override.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
token.go Introduces InlineQuestion value type and inlineQuestionValue container.
parser.go Adds inline #question parsing in collectValue() and makes legacy #question default optional.
action.go Collects inline questions during action generation; skips emitting params for question-backed arguments; adjusts dot-threshold.
shortcutgen.go Generates import questions after actions; merges legacy+inline; omits DefaultValue when empty; resets inlineQuestions.
cherri_test.go Resets inlineQuestions between tests to avoid cross-test leakage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread parser.go Outdated
Comment thread cherri_test.go
Comment thread parser.go Outdated
Comment thread parser.go
Comment thread shortcutgen.go

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread parser.go
@maximilianspitzer

Copy link
Copy Markdown
Contributor Author

@electrikmilk 👀

@electrikmilk electrikmilk self-requested a review March 22, 2026 16:26
@electrikmilk

Copy link
Copy Markdown
Owner

I touched on this in another comment, but it would be more efficient to modify the existing import question implementation to allow generating the definition and using it it then and there. We have a global actionIndex for determining the action index a import question belongs to, we should use that instead of determining it later.

@electrikmilk

electrikmilk commented Mar 22, 2026

Copy link
Copy Markdown
Owner

This is a neat idea! Makes me wonder if we should consider refactoring import questions to only support argument syntax since they can only ever be used once in one action argument based on their action index configuration.

@electrikmilk electrikmilk added the enhancement New feature or request label Mar 22, 2026
Comment thread action.go Outdated
Comment thread parser.go Outdated
Comment thread shortcutgen.go
shortcut.WFWorkflowOutputContentItemClasses = generateOutputContentItems()
},
func() {
shortcut.WFWorkflowImportQuestions = generateImportQuestions()

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why was this moved?

@electrikmilk electrikmilk Mar 22, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think I understand we need to generate actions first, I'd prefer if we didn't need to, though

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, inline questions were being collected during makeAction() so it had to run after. I'm moving the resolution into questionArg() now which is cleaner. The ordering might still need to stay this way since questionArg() runs during the same pass, but I'll see if I can avoid it 🙌

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We should be able to move this back now that we derive the action index in questionArg().

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I tested this with your branch, and putting this back into the concurrent generation is fine since we already have the actionIndex from questionArg().

Comment thread parser.go
Comment thread shortcutgen.go Outdated
Comment thread action.go Outdated
Comment thread shortcutgen.go Outdated
@maximilianspitzer

Copy link
Copy Markdown
Contributor Author

This is a neat idea! Makes me wonder if we should consider refactoring import questions to only support argument syntax since they can only ever be used once in one action argument based on their action index configuration.

@electrikmilk Thanks! Yeah I'd be down to explore deprecating the variable syntax in a follow-up if you want to go that direction 🚀

@maximilianspitzer maximilianspitzer force-pushed the feature/inline-import-questions branch 2 times, most recently from 530c10b to 353abac Compare March 23, 2026 20:46
@maximilianspitzer maximilianspitzer force-pushed the feature/inline-import-questions branch from 353abac to 1dec942 Compare March 24, 2026 20:52
@maximilianspitzer

Copy link
Copy Markdown
Contributor Author

Should all be resolved now. Take a look whenever you aren't busy. :)

@maximilianspitzer maximilianspitzer force-pushed the feature/inline-import-questions branch from 1dec942 to 7cfe667 Compare March 25, 2026 14:59
@maximilianspitzer

Copy link
Copy Markdown
Contributor Author

@electrikmilk Bump :)

Comment thread parser.go Outdated
@maximilianspitzer maximilianspitzer force-pushed the feature/inline-import-questions branch from 7cfe667 to 0179346 Compare April 1, 2026 10:29

@electrikmilk electrikmilk left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The questions can be generated concurrently before the actions due to the index being set on the argument collection.

@maximilianspitzer maximilianspitzer force-pushed the feature/inline-import-questions branch from 1c270ca to c02427c Compare April 27, 2026 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants