Skip to content

Add language filter panel.#2935

Merged
starcke merged 7 commits intomainfrom
starcke/language-selection-panel
Oct 16, 2023
Merged

Add language filter panel.#2935
starcke merged 7 commits intomainfrom
starcke/language-selection-panel

Conversation

@starcke
Copy link
Copy Markdown
Contributor

@starcke starcke commented Oct 10, 2023

This adds a language selection panel to the extension. This hooks into the filtering, so that selecting an item in the panel filters to that language.

This is the first time I added a new panel so I am quite unsure whether it is done in the right way, feedback is very welcome.

Few notes:

  • Is it enough to feature flag in package.json so that the panel is never shown or do we need to do it in extension.ts as well?
  • I could also have read the languageContext in the getTreeItem method instead of passing in selected. However I wasnt sure how often it would call getTreeItem and it seemed a bit nicer to just set it on construction, but I am happy to change it.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@starcke starcke force-pushed the starcke/language-selection-panel branch from a2a7130 to c5810fa Compare October 10, 2023 12:14
@starcke starcke force-pushed the starcke/language-selection-panel branch from c5810fa to c668b39 Compare October 10, 2023 12:34
@starcke starcke marked this pull request as ready for review October 11, 2023 11:17
@starcke starcke requested a review from a team as a code owner October 11, 2023 11:17
Copy link
Copy Markdown
Member

@koesie10 koesie10 left a comment

Choose a reason for hiding this comment

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

Is it enough to feature flag in package.json so that the panel is never shown or do we need to do it in extension.ts as well?

This should be enough, the extension.ts doesn't really have any way of controlling whether a panel is shown or not.

I could also have read the languageContext in the getTreeItem method instead of passing in selected. However I wasnt sure how often it would call getTreeItem and it seemed a bit nicer to just set it on construction, but I am happy to change it.

I think this is fine, another alternative would be to construct the tree items in getChildren, but I don't think that has any advantages over this approach either.

Comment thread extensions/ql-vscode/src/common/commands.ts
Comment thread extensions/ql-vscode/src/language-context-store.ts Outdated
Comment thread extensions/ql-vscode/src/language-context-store.ts
@starcke
Copy link
Copy Markdown
Contributor Author

starcke commented Oct 13, 2023

Addressed comments.

Copy link
Copy Markdown
Member

@koesie10 koesie10 left a comment

Choose a reason for hiding this comment

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

Just some small stylistic comments, but the code in general looks good.

Comment thread extensions/ql-vscode/src/language-context-store.ts Outdated
starcke and others added 2 commits October 13, 2023 16:00
Co-authored-by: Koen Vlaswinkel <koesie10@users.noreply.github.com>
@starcke starcke merged commit 3cbaa5a into main Oct 16, 2023
@starcke starcke deleted the starcke/language-selection-panel branch October 16, 2023 07:42
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.

2 participants