Skip to content

Support async custom completion functions for AsyncParsableCommand via async/await#855

Open
rgoldberg wants to merge 3 commits intoapple:mainfrom
rgoldberg:852-async-custom-completions
Open

Support async custom completion functions for AsyncParsableCommand via async/await#855
rgoldberg wants to merge 3 commits intoapple:mainfrom
rgoldberg:852-async-custom-completions

Conversation

@rgoldberg
Copy link
Contributor

@rgoldberg rgoldberg commented Jan 16, 2026

  • Support async custom completion functions for AsyncParsableCommand via async/await.
    • Resolve Hangs for async custom completions #852.
    • No longer use DispatchSemaphore, so more modern standards compliant & will work for platforms that do not have DispatchSemaphore.
    • The existing DispatchSemaphore implementation hangs when any async custom completions run if the root command is a sync ParsableCommand; async custom completions do not hang because of DispatchSemaphore if the root command is an AsyncParsableCommand, regardless if the command containing the property for the completing argument is a sync ParsableCommand or an AsyncParsableCommand.
    • Async custom completions are thus not supported for root ParsableCommand.
  • Make some related code more concise & consistent.
  • Improve ParsableArguments#_errorLabel DocC (unrelated, but just noticed it should be improved).

A few potential issues with / questions about what you want from this PR:

  • Async custom completions are supported only for AsyncParsableCommand root commands.
    • That seems OK to me because:
      • The existing implementation doesn't support root sync ParsableCommand.
      • I doubt there's any penalty to switch a root command from ParsableCommand to AsyncParsableCommand, since Swift Concurrency must be available, since the code is trying to use an async function for completion.
  • If the root command is a sync ParsableCommand, a ParserError.invalidState is thrown during runtime when completion candidates are requested from an async custom completion function.
    • Do you want this detected at build time or completion script generation time?
      • If so, are there any utilities to facilitate detection at build time or completion script generation time?
    • Do you want a different error?
  • Is the minor cleanup for tangentially related code acceptable here?
  • Is the unrelated DocC update acceptable here?

Checklist

  • I've added at least one test that validates that my change is working, if appropriate
  • I've followed the code style of the rest of the project
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary

@rgoldberg rgoldberg marked this pull request as ready for review January 16, 2026 13:05
@rgoldberg rgoldberg force-pushed the 852-async-custom-completions branch from b258097 to 5abed1e Compare January 18, 2026 14:03
@rgoldberg
Copy link
Contributor Author

Are there any changes that you would like for this PR?

@rgoldberg rgoldberg mentioned this pull request Mar 12, 2026
4 tasks
@natecook1000
Copy link
Member

If the root command is a sync ParsableCommand, a ParserError.invalidState is thrown during runtime when completion candidates are requested from an async custom completion function.

This should be detected at build time, if possible. There are a series of validators in the Validators directory that you can add to – they run on every execution in debug mode and effectively act as runtime compilation errors.

@rgoldberg
Copy link
Contributor Author

Thanks. I'll look into Validators soon, then add the check.

@rgoldberg rgoldberg force-pushed the 852-async-custom-completions branch 3 times, most recently from 613d0b2 to a0a6d77 Compare March 19, 2026 16:21
@rgoldberg
Copy link
Contributor Author

rgoldberg commented Mar 19, 2026

@natecook1000 I've added AsyncCompletionsValidator & tests for it.

I've rebased on current main.

Is there any way in the longterm to change SAP to run some or all of the ParsableArgumentsValidators via build plugins, so they always run at build time (for debug, release, whatever…) instead of requiring subsequent tests on a debug build?

If possible, I'll log an issue.

@rgoldberg rgoldberg force-pushed the 852-async-custom-completions branch from a0a6d77 to ed816dd Compare March 20, 2026 12:25
@natecook1000 natecook1000 added the additive change A change that requires a minor version increase label Mar 20, 2026
@rgoldberg rgoldberg force-pushed the 852-async-custom-completions branch 2 times, most recently from de32339 to 735d19c Compare March 20, 2026 19:56
Signed-off-by: Ross Goldberg <484615+rgoldberg@users.noreply.github.com>
Signed-off-by: Ross Goldberg <484615+rgoldberg@users.noreply.github.com>
…a async/await.

Signed-off-by: Ross Goldberg <484615+rgoldberg@users.noreply.github.com>
@rgoldberg rgoldberg force-pushed the 852-async-custom-completions branch from 735d19c to 858faf7 Compare March 20, 2026 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

additive change A change that requires a minor version increase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hangs for async custom completions

2 participants