Skip to content

Conversation

@rgoldberg
Copy link
Contributor

@rgoldberg rgoldberg commented Nov 1, 2024

Add singleton to indicate which shell is requesting completion candidates.

A CompletionShell singleton named CompletionShell.requesting has been created that indicates which shell is requesting completion candidates.

The singleton is populated when a completion script is generated, so functions used to generate arguments for CompletionKind creation functions can read the singleton to be able to return completion candidates / shell commands tailored for the requesting shell.

For the custom(:) CompletionKind creation function, the singleton is populated at runtime (when a completion script requests completions from the Swift app after a user types tab while composing a command line to call the app).

The requesting shell is communicated to the Swift app via an environment variable named SAP_SHELL, which is exported by each of the generated completion scripts.

Resolve #672

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 force-pushed the 672-completion-per-shell branch 2 times, most recently from bfe66ae to 21ead87 Compare November 1, 2024 13:47
@rgoldberg rgoldberg force-pushed the 672-completion-per-shell branch from 21ead87 to 30e0f78 Compare November 24, 2024 11:10
@rgoldberg
Copy link
Contributor Author

@rauhul @natecook1000 What can I do to move this forward?

Thanks.

@rgoldberg rgoldberg force-pushed the 672-completion-per-shell branch from 30e0f78 to 71458a1 Compare December 5, 2024 07:06
@rgoldberg
Copy link
Contributor Author

@rauhul @natecook1000 Are there any changes that you'd like made to this PR?

I'm happy to modify or completely redo this PR if you have a better way to indicate to Swift code which shell is requesting completions.

Thanks for any guidance.

…ates

A CompletionShell singleton named CompletionShell.requesting has been created
that indicates which shell is requesting completion candidates.

The singleton is populated when a completion script is generated, so functions
used to generate arguments for CompletionKind creation functions can return
completion candidate syntax / shell commands tailored for that shell.

For the custom(:) CompletionKind creation function, the singleton is populated
at runtime (when a completion script requests completions from the Swift app
after a user types tab while composing a command line to call the app).

The requesting shell is communicated to the Swift app via an environment variable
named SAP_SHELL, which is exported by each of the generated completion scripts.

Resolve apple#672

Signed-off-by: Ross Goldberg <[email protected]>
@rgoldberg rgoldberg force-pushed the 672-completion-per-shell branch from 71458a1 to 1941bb6 Compare January 10, 2025 03:19
@rgoldberg
Copy link
Contributor Author

Is there any way to get feedback on this PR so I can modify it however you prefer (if you want changes), then get it approved & merged?

@rauhul rauhul requested a review from natecook1000 January 12, 2025 19:07
Copy link
Member

@natecook1000 natecook1000 left a comment

Choose a reason for hiding this comment

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

Apologies for the delay, @rgoldberg! Thanks so much for both identifying the issue and building a source-compatible fix. I have just one note about the documentation; otherwise, this looks great to merge.


/// An instance representing the shell for which completions are being
/// requested.
public internal(set) static var requesting: CompletionShell?
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a note to the documentation that this is only non-nil while generating shell scripts and custom completions?

@natecook1000
Copy link
Member

@swift-ci Please test

@rgoldberg
Copy link
Contributor Author

@natecook1000 No problem about the delay. Sorry if I pestered. Thanks for the feedback.

While I have your attention, can I ask you for feedback about #679 (comment)?

I have a WIP PR that improves completion scripts in many ways, but I want to ensure that I support all versions of the shells that you want to be supported. It will take me a little while to finish that PR, but it's definitely progressed a long way.

Thanks again.

@natecook1000
Copy link
Member

@swift-ci Please test

@rgoldberg
Copy link
Contributor Author

@natecook1000 Any way to kick off the tests again?

Once this is merged, I will submit my PR for #689.

In a few weeks or so, I will submit my PR for #679.

Thanks for the help.

@natecook1000
Copy link
Member

@swift-ci Please test

@natecook1000 natecook1000 merged commit 2edadce into apple:main Jan 22, 2025
2 checks passed
@natecook1000
Copy link
Member

@rgoldberg Merged, thank you!

@rgoldberg rgoldberg deleted the 672-completion-per-shell branch January 22, 2025 06:02
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.

Custom completion per shell (zsh, bash, or fish)

2 participants