Add the option to be able to change the table format#405
Conversation
For example, from Markdown to Default (no preference) to Box-drawn
📝 WalkthroughWalkthroughAdds a new misc setting Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as MiscView
participant Config
participant Patcher as writeTableFormat
participant ThemeApplier
participant FS as Filesystem
User->>UI: toggle tableFormat
UI->>Config: update settings.misc.tableFormat
User->>UI: trigger apply customization
UI->>FS: read target file content
UI->>Patcher: writeTableFormat(oldContent, tableFormat)
Patcher-->>UI: patchedContent or null
UI->>FS: write patchedContent (if returned)
UI->>ThemeApplier: apply themes to updated content
ThemeApplier->>FS: write final output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/patches/index.ts`:
- Around line 542-546: The code reads config.settings.misc.tableFormat and can
throw when settings.misc is missing; change the guard to safely read the value
with optional chaining/defaulting (e.g., derive a tableFormat variable from
config.settings?.misc?.tableFormat ?? 'default') and then only call
writeTableFormat(content, tableFormat) if tableFormat !== 'default'; update the
block around writeTableFormat and the variables result/content handling
accordingly (referencing writeTableFormat, config.settings,
config.settings.misc.tableFormat, content, and result).
In `@src/patches/tableFormat.ts`:
- Around line 131-145: The regex using \w in tableFunctionPattern and the nearby
identifier matches (e.g., tableFunctionPattern, funcMatch usage, and the region
replacements that target let (\w) = "│"; and similar patterns around lines
176-180) will miss minified identifiers containing $, so update those identifier
capture classes to use [$\w]+ instead of \w (e.g., change
/function\s+(\w)\((\w)\)/ and /let\s+(\w)\s*=\s*"│";/ and any other /(\w)/
captures in this patch to /([$\w]+)/ or equivalent) so the
searchRegion/patchedRegion logic correctly finds and replaces vertical border
and row-separator variables for minified builds.
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/patches/tableFormat.ts`:
- Around line 200-217: Remove the unused oldLength variable and update the
showDiff invocation to pass the actual changed substring as injectedText rather
than the descriptive message: compute injectedText using the same
contextStart/contextEnd (e.g., newFile.slice(contextStart, contextEnd) or the
exact changed segment around firstDiffIndex) and pass that as the injectedText
argument to showDiff, keeping the human-readable description separate (e.g., as
a title or log) so the length-based calculations inside showDiff (which use
injectedText.length) are correct; locate symbols findFirstDifference,
contextStart, contextEnd, patchCount, injectedText, and showDiff to make the
change.
🧹 Nitpick comments (3)
CHANGELOG.md (1)
11-11: Consider adding the issue reference for traceability.The PR description mentions this addresses issue
#113. Adding the issue number would be consistent with other entries (e.g., line 10) and help users trace the feature request.-- Add the option to be able to change the table format - `@mike1858` +- Add the option to be able to change the table format (`#113`) - `@mike1858`README.md (2)
172-180: Add language specifier to fenced code block.Per markdownlint, fenced code blocks should have a language specified. Since this is plain text output, consider using
textorplaintext.Suggested fix
-``` +```text ┌───────┬─────┬───────────────┐
184-189: Add language specifier to fenced code block.Same as above - consider using
textormarkdownfor the markdown table example.Suggested fix
-``` +```text | Name | Age | City |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@README.md`:
- Line 50: The README uses a lowercase "markdown" in several places; update
those instances to capitalized "Markdown" to treat it as a proper noun. Locate
the sentence starting "Switch between **table formats**: keep the default
Unicode..." (the segment showing "`| A | B |`") and replace "markdown tables"
with "Markdown tables", and do the same for the other occurrences matching the
lowercase "markdown" text highlighted in the comment (the other two/three
occurrences). Ensure only the capitalization changes and preserve surrounding
formatting and backticks.
🧹 Nitpick comments (1)
README.md (1)
172-207: Add language identifiers to fenced code blocks.Markdownlint flags missing languages for these fences. Add a language (e.g.,
text) to improve lint compliance and readability.♻️ Proposed fix
-``` +```text ┌───────────┬───────────────────────────────┬───────┐ │ Library │ Purpose │ Size │ ├───────────┼───────────────────────────────┼───────┤ │ React │ UI components, virtual DOM │ ~40kb │ ├───────────┼───────────────────────────────┼───────┤ │ Vue │ Progressive framework │ ~34kb │ ├───────────┼───────────────────────────────┼───────┤ │ Svelte │ Compile-time framework │ ~2kb │ ├───────────┼───────────────────────────────┼───────┤ │ Preact │ Lightweight React alternative │ ~3kb │ ├───────────┼───────────────────────────────┼───────┤ │ Solid │ Reactive UI, no virtual DOM │ ~7kb │ ├───────────┼───────────────────────────────┼───────┤ │ htmx │ HTML-driven interactivity │ ~14kb │ ├───────────┼───────────────────────────────┼───────┤ │ Alpine.js │ Lightweight reactivity │ ~15kb │ ├───────────┼───────────────────────────────┼───────┤ │ Lit │ Web components │ ~5kb │ └───────────┴───────────────────────────────┴───────┘ -``` +``` -``` +```text | Library | Purpose | Size | |-----------|-------------------------------|-------| | React | UI components, virtual DOM | ~40kb | | Vue | Progressive framework | ~34kb | | Svelte | Compile-time framework | ~2kb | | Preact | Lightweight React alternative | ~3kb | | Solid | Reactive UI, no virtual DOM | ~7kb | | htmx | HTML-driven interactivity | ~14kb | | Alpine.js | Lightweight reactivity | ~15kb | | Lit | Web components | ~5kb | -``` +```
Are the columns still aligned in properly in both cases? I don't mind the nice clean lines (I'm not necessarily looking for pipes and dashes), but I do want to remove the horizontal rule between each item because it doubles the height of the table, and it makes the table harder to read. The vertical rules are useful, and perhaps even the rule below the headings is useful, but the rule between the items is excessive. ie, just omit What do you think? Does this cover that? |
|
@KJ7LNW Yes, I think it's exactly what you're looking for. With the "markdown" option, it goes from: To: |
Yes? ;) Honestly I could see it be useful in either direction. The borders are nice. But the vertical compactness is nice to drop the top and bottom lines. Maybe you can make an executive decision on that one based on which you prefer. In my humble opinion, dropping the top and the bottom lines is preferred because it maps on to the raw markdown output. |
Expand table format options from 2 to 4 variants: - default: original box-drawing with all row separators - ascii: markdown-style using | and - (renamed from 'markdown') - clean: box-drawing without borders or row separators - clean-top-bottom: box-drawing with top/bottom borders, no row separators
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@README.md`:
- Around line 174-218: The fenced table examples in README.md (the initial box
and the three formats labeled "ascii", "clean", and "clean-top-bottom") lack
language identifiers and trigger markdownlint MD040; update each fenced code
block to include the language identifier `text` (i.e., change ``` to ```text) so
the table content is treated as literal text and the lint warning is resolved.
In `@src/patches/tableFormat.ts`:
- Around line 89-107: The clean-border constants (TABLE_BORDERS_CLEAN /
TABLE_BORDERS_CLEAN_NATIVE / TABLE_BORDERS_CLEAN_SPACED) define empty strings
for top/bottom which leads to blank rows because the renderer unconditionally
calls T("top") and T("bottom") (see the R.push(T("top")) / R.push(T("bottom"))
usage); fix by either skipping pushing top/bottom when using the clean border
set (detect via the TABLE_BORDERS_CLEAN symbol or when T("top")/T("bottom")
returns an empty string) or by filtering out any empty row results before adding
to the output array (i.e., only push non-empty strings from T(...)); update the
renderer where R.push(T("top"))/R.push(T("bottom")) occurs to implement one of
these approaches so clean format no longer produces blank lines.
In `@src/ui/components/MiscView.tsx`:
- Around line 60-70: The displayed labels in getTableFormatDisplay misrepresent
the "clean" formats (they do still show vertical borders and the header rule);
update the return strings for the 'clean' and 'clean-top-bottom' cases in
getTableFormatDisplay to accurately describe their output (e.g., mention that
they keep vertical borders and the header separator or have only top/bottom
header rule), and make the identical wording change in the other table-format
label mapping in the same file so both places show the corrected descriptions.
♻️ Duplicate comments (1)
README.md (1)
50-50: Capitalize “Markdown” in user-facing text.
Line 50 and Line 170 refer to the Markdown language but use lowercase.✏️ Suggested edit
- - Switch between **table formats**: keep the default Unicode box-drawing characters (`┌─┬─┐`), use **ASCII/markdown tables** (`| A | B |`) for easier copy-paste, or use **clean** variants without row separators. + - Switch between **table formats**: keep the default Unicode box-drawing characters (`┌─┬─┐`), use **ASCII/Markdown tables** (`| A | B |`) for easier copy-paste, or use **clean** variants without row separators. - Recent Claude Code versions render tables using Unicode box-drawing characters. While these have a more elegant look compared to the traditional plain markdown table rendering, they take up more room due to the row dividers: + Recent Claude Code versions render tables using Unicode box-drawing characters. While these have a more elegant look compared to the traditional plain Markdown table rendering, they take up more room due to the row dividers:Also applies to: 170-170
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/patches/tableFormat.ts`:
- Around line 233-313: The ASCII branch leaves T("top")/T("bottom") pushes in
place causing blank top/bottom rows; after applying TABLE_BORDERS_ASCII (and
related replacements) call removeTopBottomPushes(newFile) and, if success, set
newFile = result, increment patchCount and emit a debug like 'Removed top/bottom
pushes for ASCII format' so the empty rows are eliminated (refer to
TABLE_BORDERS_ASCII, removeTopBottomPushes, and the ascii handling block).
♻️ Duplicate comments (1)
README.md (1)
50-50: Capitalize "Markdown" as a proper noun.The term "Markdown" refers to the markup language and should be capitalized:
- Line 50: "ASCII/markdown tables" → "ASCII/Markdown tables"
- Line 170: "plain markdown table rendering" → "plain Markdown table rendering"
📝 Proposed fixes
Line 50:
-- Switch between **table formats**: keep the default Unicode box-drawing characters (`┌─┬─┐`), use **ASCII/markdown tables** (`| A | B |`) for easier copy-paste, or use **clean** variants without row separators. +- Switch between **table formats**: keep the default Unicode box-drawing characters (`┌─┬─┐`), use **ASCII/Markdown tables** (`| A | B |`) for easier copy-paste, or use **clean** variants without row separators.Line 170:
-Recent Claude Code versions render tables using Unicode box-drawing characters. While these have a more elegant look compared to the traditional plain markdown table rendering, they take up more room due to the row dividers: +Recent Claude Code versions render tables using Unicode box-drawing characters. While these have a more elegant look compared to the traditional plain Markdown table rendering, they take up more room due to the row dividers:Also applies to: 170-170
🧹 Nitpick comments (1)
src/ui/components/MiscView.tsx (1)
304-310: Optional: cachegetValue()for clarity and consistency.
This avoids repeated calls and keeps the display consistent if a getter ever becomes non-trivial.♻️ Proposed refactor
const actualIndex = scrollOffset + i; const isSelected = actualIndex === selectedIndex; const isMultiValue = item.isMultiValue ?? false; - const checkbox = isMultiValue ? '◉' : item.getValue() ? '☑' : '☐'; + const value = item.getValue(); + const checkbox = isMultiValue ? '◉' : value ? '☑' : '☐'; const statusText = isMultiValue - ? (item.getDisplayValue?.() ?? String(item.getValue())) - : item.getValue() + ? (item.getDisplayValue?.() ?? String(value)) + : value ? 'Enabled' : 'Disabled';Also applies to: 334-334
|
@KJ7LNW I ended up adding both options:
|
|
Awesome, Thank you for putting that together. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@README.md`:
- Line 247: The README uses lowercase "markdown" in the sentence about table
rendering; change that token to "Markdown" to match the capitalized usage
elsewhere (e.g., the occurrence noted at line 265) so the term is consistently
treated as a proper noun; search for the exact string "markdown" in the
surrounding paragraph and replace it with "Markdown".
♻️ Duplicate comments (1)
README.md (1)
251-261: Note: Code block language identifier omitted.The fenced code blocks in this section lack language identifiers (flagged by markdownlint MD040). However, based on past review feedback on similar suggestions, this was deemed unnecessary for table examples.
🧹 Nitpick comments (2)
README.md (2)
59-59: Clarify the table format options in the feature description.The current description doesn't clearly enumerate the four available formats (
default,ascii,clean,clean-top-bottom) and is somewhat confusing (e.g., "Claude Code default" vs "Unicode" when default is Unicode box-drawing).📝 Suggested improvement
-- Switch between **table formats** - Claude Code default, Unicode (`┌─┬─┐`), ASCII/markdown (`|---|`), Unicode without top/bottom borders. +- Switch between **table formats**: `default` (full Unicode box-drawing), `ascii` (Markdown-style `|---|`), `clean` (box-drawing without top/bottom borders), or `clean-top-bottom` (box-drawing with top/bottom, no row separators).
275-283: Clarify "row separators" terminology.The descriptions for
clean(line 275) andclean-top-bottom(line 285) mention "no row separators," but the examples show that the header separator line (the line after the header row) is retained. Consider clarifying as "no intermediate row separators" or "no row separators between data rows" to avoid ambiguity.📝 Suggested improvements
For line 275:
-**`clean`** — Box-drawing without top/bottom borders or row separators: +**`clean`** — Box-drawing without top/bottom borders or intermediate row separators:For line 285:
-**`clean-top-bottom`** — Box-drawing with top/bottom borders but no row separators: +**`clean-top-bottom`** — Box-drawing with top/bottom borders but no intermediate row separators:Also applies to: 285-295






For example, from Markdown to Default (no preference) to Box-drawn.
Requested in #113 (comment).
Summary by CodeRabbit
New Features
Behavior
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.