Skip to content

Add helper for substituting config variables#3530

Merged
koesie10 merged 1 commit intomainfrom
koesie10/config-template
Apr 5, 2024
Merged

Add helper for substituting config variables#3530
koesie10 merged 1 commit intomainfrom
koesie10/config-template

Conversation

@koesie10
Copy link
Copy Markdown
Member

@koesie10 koesie10 commented Apr 3, 2024

This adds a helper for substituting config variables, such as the following:

  • .github/codeql/extensions/${name}-${language} becomes .github/codeql/extensions/vscode-codeql-java
  • ${owner}/${name}-${language} becomes github/vscode-codeql-java
  • ${workspaceFolder}${pathSeparator}.github/workflows/codeql-analysis.yml becomes /home/your-username/your-project/.github/workflows/codeql-analysis.yml

This is based on the implementation in VS Code that is used for the window.title setting. Some of the unit tests are also based on the VS Code unit tests.

This helper function will be used in follow-up PRs that introduce new settings for model packs.

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.

@koesie10 koesie10 force-pushed the koesie10/config-template branch from ccf5450 to 8cb2b34 Compare April 3, 2024 12:43
@koesie10 koesie10 marked this pull request as ready for review April 3, 2024 14:13
@koesie10 koesie10 requested a review from a team as a code owner April 3, 2024 14:13
@koesie10 koesie10 requested a review from a team April 3, 2024 14:14
Copy link
Copy Markdown
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

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

Implementation LGTM. Thanks for including the link to the vscode repo.

Side question: as well as the new model packs uses, do you plan to replace the interpolation in HistoryItemLabelProvider with this implementation?

@koesie10
Copy link
Copy Markdown
Member Author

koesie10 commented Apr 5, 2024

Side question: as well as the new model packs uses, do you plan to replace the interpolation in HistoryItemLabelProvider with this implementation?

I don't think we can without breaking existing settings since that uses a format like %q on %d - %s %r [%t] which is incompatible with this "standard" VS Code format. We might be able to deprecate the old format and switch to this new format, but I don't think that's something we're looking into right now.

@charisk
Copy link
Copy Markdown
Contributor

charisk commented Apr 5, 2024

Side question: as well as the new model packs uses, do you plan to replace the interpolation in HistoryItemLabelProvider with this implementation?

I don't think we can without breaking existing settings since that uses a format like %q on %d - %s %r [%t] which is incompatible with this "standard" VS Code format. We might be able to deprecate the old format and switch to this new format, but I don't think that's something we're looking into right now.

Might be worth an issue to track this as potential tech/product debt.

@koesie10 koesie10 merged commit 351bc64 into main Apr 5, 2024
@koesie10 koesie10 deleted the koesie10/config-template branch April 5, 2024 09:15
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