Skip to content
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

Display output channel when addons are clicked #2850

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Nov 14, 2024

This way, when users see that an addon is errored, they can quickly navigate to the output channel to see the error.

Motivation

Closes https://github.com/Shopify/team-ruby-dx/issues/1324

Implementation

Since the Output tab is the only place we display information about addons now, I made the addon quick pick items clickable to navigate to it.

Currently clicking any addon will always go to the same Ruby LSP output channel. But if we started creating new channels for individual addons, we can provide a better experience by pointing to addon-specific channels too.

Automated Tests

Manual Tests

This way, when users see that an addon is errored, they can quickly
navigate to the output channel to see the error.
@st0012 st0012 added enhancement New feature or request vscode This pull request should be included in the VS Code extension's release notes labels Nov 14, 2024
@st0012 st0012 self-assigned this Nov 14, 2024
@st0012 st0012 requested a review from a team as a code owner November 14, 2024 20:25

return -1;
})
.sort((addon) => (addon.errored ? 1 : -1))
Copy link

Choose a reason for hiding this comment

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

The sort callback needs to handle the case where both addons have the same error state. The current implementation only looks at a single addon at a time, which isn't sufficient for a proper sort. Here's the correct implementation:

.sort((a, b) => (a.errored === b.errored ? 0 : a.errored ? 1 : -1))

This ensures stable sorting by comparing both addons and maintaining their relative positions when they share the same error state.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.


return -1;
})
.sort((addon) => (addon.errored ? 1 : -1))
Copy link

Choose a reason for hiding this comment

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

The sort callback needs to handle the case where both addons have the same error state. The current implementation only looks at a single addon at a time, which isn't sufficient for a proper sort. Here's the correct implementation:

.sort((a, b) => (a.errored === b.errored ? 0 : a.errored ? 1 : -1))

This ensures stable sorting by comparing both addons and maintaining their relative positions when they share the same error state.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.


quickPick.onDidAccept(() => {
const selected = quickPick.selectedItems[0];
// Ideally, we should display information that's specific to the selected addon
Copy link
Member

Choose a reason for hiding this comment

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

If we provide an API in the server that tags logs with the add-on name, then you might be able to reveal the output channel with a filter applied, so that you only see the output related to that add-on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request vscode This pull request should be included in the VS Code extension's release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants