Skip to content

feat: workspace open settings #1055

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

Merged
merged 6 commits into from
Apr 22, 2025
Merged

feat: workspace open settings #1055

merged 6 commits into from
Apr 22, 2025

Conversation

volodkevych
Copy link
Contributor

Problem

There was no way to prompt customer to enable workspace indexing lib.

Solution

Show button to prompt customer to enable local workspace indexing.
Extension support of OPEN_SETTINGS is needed.

Screen.Recording.2025-04-22.at.18.48.47.mov

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@volodkevych volodkevych requested a review from a team as a code owner April 22, 2025 16:53
@volodkevych volodkevych force-pushed the workspace-open-settings branch from 97c259a to 9eeb19a Compare April 22, 2025 16:56
if (!localProjectContextController.isEnabled) {
// TODO: Prompt user to enable indexing
if (!localProjectContextController.isEnabled && chatResultStream) {
await chatResultStream.writeResultBlock({
Copy link
Contributor

@viktorsaws viktorsaws Apr 22, 2025

Choose a reason for hiding this comment

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

It feels a bit weird to stuff this message so deep in the triggerContext extracting utility. Given that we have access to context commands in params passed here and localProjectContextController in agenticChatController, this check for message could be done in agenticChatController itself

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 agree, but this code block depends on data parsed in this trigger context, so the simplest was to put it here. I could decouple it better, if I had more time.

Copy link
Contributor

Choose a reason for hiding this comment

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

I approved, we can take it as a follow-up. I didn't see which parsed data it needs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It depends on useRelevantDocuments, but yes, I will be doing another improvement, so can improve this one too.

@volodkevych volodkevych merged commit f3018da into main Apr 22, 2025
6 checks passed
@volodkevych volodkevych deleted the workspace-open-settings branch April 22, 2025 17:30
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.

3 participants