Allow toolbar buttons visibility to be configured via settings#1598
Allow toolbar buttons visibility to be configured via settings#1598matthieucan wants to merge 6 commits intoAltimateAI:masterfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds seven new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Editor
participant Config as Config Checker
participant Cmd as Command Executor
User->>Editor: Open SQL file / click editor title action
Editor->>Config: Query `config.dbt.<feature>` and language (sql/jinja-sql)
Config-->>Editor: Return true/false
alt config true and language matches
Editor->>Cmd: Invoke command (e.g., execute/run/test/convert)
Cmd-->>User: Command executes / feedback shown
else config false or language mismatch
Editor-->>User: Action hidden / not available
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 253ae83 in 2 minutes and 44 seconds
More details
- Looked at
124lines of code in1files - Skipped
0files when reviewing. - Skipped posting
6drafted comments based on config settings.
1. package.json:229
- Draft comment:
New configuration settings for toolbar button visibility are added correctly. Consider updating the README or documentation to describe these new options, so users know how to customize the toolbar. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is purely informative and suggests updating documentation, which is against the rules. It doesn't provide a specific code suggestion or highlight a potential issue with the code itself.
2. package.json:868
- Draft comment:
When conditions in the menu contributions now include configuration checks. To reduce repetitive code, consider extracting the common language regex (resourceLangId =~ /^sql$|^jinja-sql$/) into a reusable constant or variable. - Reason this comment was not posted:
Comment was on unchanged code.
3. package.json:958
- Draft comment:
Updated 'editor/title/run' menu commands include config checks for build model actions. Ensure that the selected configuration key (e.g., config.dbt.enableBuildModel) is used consistently with your intended behavior, especially since there are separate settings for running a model vs building a model. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the author to ensure that a configuration key is used consistently with the intended behavior. This is a request for confirmation of intention, which violates the rules. The comment does not provide a specific code suggestion or ask for a specific test to be written. Therefore, it should be removed.
4. package.json:232
- Draft comment:
New configuration properties for toggling toolbar buttons are added here. Please update the README/documentation to describe these new settings so users know how to control toolbar visibility. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to update the documentation, which is not allowed according to the rules. The comment is purely informative and does not provide a specific code suggestion or ask for a test to be written.
5. package.json:869
- Draft comment:
Toolbar command entries now consistently append configuration checks (e.g., '&& config.dbt.enableExecuteSql'). Verify that using 'dbt.enableBuildModel' for both model and project build commands is the intended behavior. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
6. package.json:262
- Draft comment:
Minor typographical inconsistency: in the description for 'dbt.enableConvertToModel', 'model' is written in lowercase ('Convert to dbt model') while similar settings use 'Model' (e.g., 'Run dbt Model', 'Test dbt Model'). Consider capitalizing it ('Convert to dbt Model') for consistency. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
Looking at the rules, UI and styling changes should be ignored. While this is technically about consistency in labels, it falls under UI styling which we're told to assume the author handled correctly. Additionally, this is a very minor issue that doesn't affect functionality.
Could inconsistent capitalization in UI elements cause confusion for users? Could this be considered a documentation issue rather than just UI styling?
While consistency is good, this is fundamentally about UI presentation and button labels, which the rules explicitly tell us to ignore. The meaning is clear regardless of capitalization.
Delete the comment as it relates to UI styling choices which we're instructed to ignore.
Workflow ID: wflow_QaPh12cvgq7qVStH
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
This is OOTB supported by vscode. I don't have big objection but these things are sometimes easy to overlook when maintaining the extension. So I prefer to not support any features that are already supported by vscode.
|
You're right! However, I think this can't be configured through |
| { | ||
| "command": "dbtPowerUser.runCurrentModel", | ||
| "when": "resourceLangId =~ /^sql$|^jinja-sql$/", | ||
| "when": "resourceLangId =~ /^sql$|^jinja-sql$/ && config.dbt.enableBuildModel", |
There was a problem hiding this comment.
That one is trickier than the others: all the actions within the single button "Build project" are behind a single flag.
This allows to display/hide the entire group (the button with the down arrow), rather than sub-items in this group. Only hiding a subset of the sub-items within this button would not hide the button itself, therefore it would be less useful IMO.
Does that make sense?
|
@mdesmet @saravmajestic We will have to update the docs also - https://docs.myaltimate.com/setup/configuration/ |
Apologies for the (long) delay in getting back to you. I have now updated the documentation, thanks for the pointer! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
package.json (1)
250-284: LGTM — new visibility settings are consistent and safe by default.The seven new boolean flags follow the existing
dbt.*config namespace, all default totrue(preserving prior behavior), and their descriptions clearly map to the corresponding toolbar buttons. Naming is uniform (enable<Feature>) and each setting is referenced by exactly onewhenclause in the menu definitions below.One minor consideration (optional): the keybindings for
dbtPowerUser.executeSQL(Lines 906-909) anddbtPowerUser.sqlPreview(Lines 911-915), as well as command-palette invocations, are not gated by these flags. That is consistent with the PR's stated scope ("toolbar button visibility"), but users who disable a button may still trigger the command via shortcut/palette. Worth a brief note in the documentation to set expectations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 250 - 284, The keybinding entries for dbtPowerUser.executeSQL and dbtPowerUser.sqlPreview are not gated by the new visibility settings, so add the corresponding when-clauses (e.g., "config.dbt.enableExecuteSql" and "config.dbt.enableSqlPreview") to those keybinding/menu definitions to match the toolbar visibility flags (or alternatively add a short docs note stating that disabling the button does not disable the keybinding/command); update the keybinding objects that reference the commands dbtPowerUser.executeSQL and dbtPowerUser.sqlPreview to include the matching "when" expressions so behavior is consistent with the menu buttons.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@package.json`:
- Around line 250-284: The keybinding entries for dbtPowerUser.executeSQL and
dbtPowerUser.sqlPreview are not gated by the new visibility settings, so add the
corresponding when-clauses (e.g., "config.dbt.enableExecuteSql" and
"config.dbt.enableSqlPreview") to those keybinding/menu definitions to match the
toolbar visibility flags (or alternatively add a short docs note stating that
disabling the button does not disable the keybinding/command); update the
keybinding objects that reference the commands dbtPowerUser.executeSQL and
dbtPowerUser.sqlPreview to include the matching "when" expressions so behavior
is consistent with the menu buttons.
✅ Tests — All Passedcc @matthieucan |
5 participants
Overview
Problem
Hi! Thanks for the great extension.
I would like the toolbar buttons to be configurable so they can be hidden. I find the long list (7 buttons) prone to misclicks, and there are some that I don't use as they are not relevant for my projects.
Solution
Make every button conditionally appear, via an associated setting. By default, they are all present, so this does not alter the current behavior.
Screenshot/Demo
A simplified toolbar with only the "Run" and "Test" buttons enabled:
How to test
Checklist
README.mdupdated and added information about my changeImportant
Toolbar buttons in SQL files are now configurable via new boolean settings in
package.json, allowing users to hide unused buttons.package.json.dbt.enableBuildModel,dbt.enableExecuteSql,dbt.enableSqlQuickPick,dbt.enableRunModel,dbt.enableTestModel,dbt.enableSqlPreview,dbt.enableConvertToModel.editor/titleandeditor/title/runcommands to check new settings before displaying buttons.This description was created by
for 253ae83. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Documentation