feat(format): make tab visualization opt-in via CLI_TAB_VISUALIZE#579
feat(format): make tab visualization opt-in via CLI_TAB_VISUALIZE#579
Conversation
Tab visualization replaces tab characters with → symbol in table output, but this affects copy-paste and text redirection. Make it opt-in by adding CLI_TAB_VISUALIZE system variable (default FALSE). - Add TabVisualize bool to DisplayVars and FormatConfig - Register CLI_TAB_VISUALIZE as boolean system variable - Guard visualizeTabsInRow call with config.TabVisualize check - Update TestWriteTable_WithTabs to set TabVisualize: true Fixes #577 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces an opt-in mechanism for tab visualization in the CLI's table output. Previously, tab characters were always replaced with Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Code Metrics Report📊 View detailed coverage report (available for 7 days)
Details | | main (f95515b) | #579 (6301d80) | +/- |
|---------------------|----------------|----------------|-------|
+ | Coverage | 70.8% | 70.8% | +0.0% |
| Files | 75 | 75 | 0 |
| Lines | 7025 | 7027 | +2 |
+ | Covered | 4980 | 4982 | +2 |
+ | Code to Test Ratio | 1:1.3 | 1:1.3 | +0.0 |
| Code | 16299 | 16305 | +6 |
+ | Test | 21212 | 21227 | +15 |
+ | Test Execution Time | 1m9s | 1m6s | -3s |Code coverage of files in pull request scope (88.3% → 88.4%)
Reported by octocov |
There was a problem hiding this comment.
Code Review
This pull request introduces a new feature to visualize tab characters in table output. It adds a TabVisualize boolean field to FormatConfig and DisplayVars, enabling conditional tab visualization in streaming_table.go. The feature is exposed as a configurable CLI system variable, CLI_TAB_VISUALIZE, and includes updated tests to cover its functionality.
There was a problem hiding this comment.
Pull request overview
This PR makes table tab visualization opt-in via a new CLI_TAB_VISUALIZE system variable (default FALSE), so table output no longer replaces tab characters with → unless explicitly enabled—improving copy/paste and redirected output fidelity.
Changes:
- Add
TabVisualizeto display/system variables and propagate it intoformat.FormatConfig. - Register
CLI_TAB_VISUALIZEas a boolean system variable and include it in system variable set/get tests. - Guard tab visualization in the table formatter and update tab visualization table tests to enable the new flag.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/mycli/var_registry.go | Registers CLI_TAB_VISUALIZE in the system variable registry. |
| internal/mycli/system_variables_test.go | Adds CLI_TAB_VISUALIZE to the boolean variables set/get test list. |
| internal/mycli/system_variables.go | Adds TabVisualize to DisplayVars and wires it into FormatConfig. |
| internal/mycli/format/config.go | Extends FormatConfig with the TabVisualize flag. |
| internal/mycli/format/streaming_table.go | Makes visualizeTabsInRow conditional on config.TabVisualize. |
| internal/mycli/format/tab_visualize_test.go | Updates WriteTable tab tests to explicitly enable visualization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address review feedback: - Move CLI_TAB_VISUALIZE registration from integer section to boolean section in var_registry.go - Add disabled_by_default test case verifying no arrow symbol when TabVisualize is false (default) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request makes tab character visualization in table output an opt-in feature controlled by the new CLI_TAB_VISUALIZE system variable. This change addresses potential issues with copy-pasting or redirecting output. The implementation involves adding a TabVisualize flag to FormatConfig, which is used to conditionally apply tab visualization. The new system variable is registered and wired into the configuration, and the associated tests have been updated to reflect the new opt-in behavior. The changes are correct and well-implemented.
|
/gemini summary |
Summary of ChangesThis pull request addresses a usability concern where automatic tab visualization in table output, while visually helpful, interfered with copy-paste and text redirection by embedding arrow symbols. The change introduces a new system variable, Highlights
Changelog
Activity
|
Summary
Make tab visualization opt-in via
CLI_TAB_VISUALIZEsystem variable (defaultFALSE). Tab visualization replaces tab characters with→symbol in table output, but this affects copy-paste and text redirection — the→symbol appears in copied/redirected text.Key Changes
TabVisualize booltoDisplayVars, wire toFormatConfigTabVisualize booltoFormatConfigvisualizeTabsInRowcall withconfig.TabVisualizecheckCLI_TAB_VISUALIZEas boolean system variableCLI_TAB_VISUALIZEto boolean variable test listTabVisualize: trueinTestWriteTable_WithTabsTest Plan
make checkpassesTestWriteTable_WithTabspasses withTabVisualize: trueTestSystemVariables_SetGetOperationsincludesCLI_TAB_VISUALIZEFixes #577