feat: add syntax highlighting for R, Ruby, PHP, and SQL#76
Conversation
Summary of ChangesHello @Psypeal, 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 enhances the application's syntax highlighting capabilities by adding support for R, Ruby, PHP, and SQL. It also integrates the syntax highlighter with the Markdown viewer, improving the readability and presentation of code blocks in assistant text. The changes include adding keyword sets and comment detection for the new languages, and ensuring SQL keyword matching is case-insensitive. Highlights
Changelog
Activity
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughThis PR enhances syntax highlighting for code blocks in the Markdown viewer by implementing per-line highlighting and extending language support to include R, Ruby, PHP, and SQL with their respective keyword sets and comment syntaxes. Changes
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/renderer/components/chat/viewers/MarkdownViewer.tsx (1)
1-35: Align import grouping with the repo ordering convention.
External package imports should appear before path aliases and relative imports; currently some externals are mixed after alias imports.♻️ Suggested import reordering
import React from 'react'; import ReactMarkdown, { type Components } from 'react-markdown'; +import { FileText } from 'lucide-react'; +import remarkGfm from 'remark-gfm'; +import { useShallow } from 'zustand/react/shallow'; import { api } from '@renderer/api'; import { CopyButton } from '@renderer/components/common/CopyButton'; import { CODE_BG, @@ PROSE_TABLE_HEADER_BG, } from '@renderer/constants/cssVariables'; import { useStore } from '@renderer/store'; -import { FileText } from 'lucide-react'; -import remarkGfm from 'remark-gfm'; -import { useShallow } from 'zustand/react/shallow'; import { createSearchContext, highlightSearchInChildren, type SearchContext, } from '../searchHighlightUtils'; import { highlightLine } from '../viewers/syntaxHighlighter';As per coding guidelines "Organize imports in order: external packages, path aliases (
@main,@renderer,@shared,@preload), then relative imports".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/chat/viewers/MarkdownViewer.tsx` around lines 1 - 35, Imports in MarkdownViewer.tsx are not ordered per repo convention; move all external package imports (e.g., React, react-markdown, lucide-react, remark-gfm, zustand/react/shallow) to the top, then group path alias imports (e.g., `@renderer/api`, `@renderer/components`, `@renderer/constants`, `@renderer/store`), and finally keep relative imports (../searchHighlightUtils, ../viewers/syntaxHighlighter) last; adjust the current import order so external packages precede alias imports and relative imports follow aliases to match the project's import ordering rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@resources/pricing.json`:
- Around line 1827-1831: The new provider_specific_entry field added to the
claude-opus-4-6 and claude-opus-4-6-20260205 entries is unused by the codebase
(the LiteLLMPricing type allows arbitrary keys but no pricing logic reads
provider_specific_entry); either remove provider_specific_entry from those JSON
entries or add a clear comment and/or a tracked follow-up issue referencing
LiteLLMPricing to justify it as future work; update the PR to either delete the
field from the two model objects or add a note in the PR and JSON indicating
it’s intentional and tracked.
In `@src/renderer/components/chat/viewers/MarkdownViewer.tsx`:
- Around line 157-161: Create and export an isString type guard in
src/renderer/utils/stringUtils.ts (export const isString = (value: unknown):
value is string => typeof value === 'string') and then update MarkdownViewer.tsx
to import it from '@renderer/utils/stringUtils' and replace the inline check in
the isBlock branch (where raw is computed from children) by using
isString(children) ? children : '' so the component uses the centralized string
utility instead of typeof checks.
---
Nitpick comments:
In `@src/renderer/components/chat/viewers/MarkdownViewer.tsx`:
- Around line 1-35: Imports in MarkdownViewer.tsx are not ordered per repo
convention; move all external package imports (e.g., React, react-markdown,
lucide-react, remark-gfm, zustand/react/shallow) to the top, then group path
alias imports (e.g., `@renderer/api`, `@renderer/components`, `@renderer/constants`,
`@renderer/store`), and finally keep relative imports (../searchHighlightUtils,
../viewers/syntaxHighlighter) last; adjust the current import order so external
packages precede alias imports and relative imports follow aliases to match the
project's import ordering rule.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
resources/pricing.jsonsrc/renderer/components/chat/viewers/MarkdownViewer.tsxsrc/renderer/components/chat/viewers/syntaxHighlighter.ts
resources/pricing.json
Outdated
| "tool_use_system_prompt_tokens": 346, | ||
| "provider_specific_entry": { | ||
| "us": 1.1, | ||
| "fast": 6 | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if provider_specific_entry is consumed anywhere in the codebase
echo "=== Searching for 'provider_specific_entry' usage ==="
rg -n 'provider_specific_entry' --type-not json
echo ""
echo "=== Searching for pricing.json imports/reads ==="
rg -n 'pricing\.json' -C2
echo ""
echo "=== Check what files import or load pricing data ==="
rg -n 'pricing' --type js --type ts --type python | head -50Repository: matt1398/claude-devtools
Length of output: 8180
🏁 Script executed:
#!/bin/bash
# Check how pricing data is consumed in pricing.ts
echo "=== Read pricing.ts to understand structure ==="
head -100 src/shared/utils/pricing.ts
echo ""
echo "=== Search for provider_specific or field access in pricing.ts ==="
rg -n '\.' src/shared/utils/pricing.ts | head -30
echo ""
echo "=== Check LiteLLMPricing interface ==="
sed -n '4,50p' src/shared/utils/pricing.tsRepository: matt1398/claude-devtools
Length of output: 5792
Remove unused provider_specific_entry field or clarify its purpose.
The addition of provider_specific_entry to claude-opus-4-6 and claude-opus-4-6-20260205 is unused — no code in the repository references this field. While the LiteLLMPricing interface permits it via its catch-all [key: string]: unknown, the field is never accessed during pricing calculations.
If this is incomplete work or preparation for future functionality, add a comment in the JSON or track it in a follow-up issue. If it was added by mistake, remove it. Additionally, this pricing schema change appears unrelated to the PR's stated objective (syntax highlighting for R, Ruby, PHP, SQL) — consider splitting it into a separate PR for clarity.
Also applies to: 1862-1866
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@resources/pricing.json` around lines 1827 - 1831, The new
provider_specific_entry field added to the claude-opus-4-6 and
claude-opus-4-6-20260205 entries is unused by the codebase (the LiteLLMPricing
type allows arbitrary keys but no pricing logic reads provider_specific_entry);
either remove provider_specific_entry from those JSON entries or add a clear
comment and/or a tracked follow-up issue referencing LiteLLMPricing to justify
it as future work; update the PR to either delete the field from the two model
objects or add a note in the PR and JSON indicating it’s intentional and
tracked.
| if (isBlock) { | ||
| const lang = codeClassName?.replace('language-', '') ?? ''; | ||
| const raw = typeof children === 'string' ? children : ''; | ||
| const text = raw.replace(/\n$/, ''); | ||
| const lines = text.split('\n'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n --type=ts --type=tsx '\bisString\b' -A 2 -B 2Repository: matt1398/claude-devtools
Length of output: 95
🏁 Script executed:
fd -e ts -e tsx | xargs grep -l "isString" | head -20Repository: matt1398/claude-devtools
Length of output: 100
🏁 Script executed:
find . -path "*/utils/stringUtils.ts" -o -path "*/utils/*string*" | head -10Repository: matt1398/claude-devtools
Length of output: 265
🏁 Script executed:
cat -n src/renderer/components/chat/viewers/MarkdownViewer.tsx | sed -n '150,170p'Repository: matt1398/claude-devtools
Length of output: 1123
🏁 Script executed:
cat src/renderer/utils/stringUtils.tsRepository: matt1398/claude-devtools
Length of output: 1820
🏁 Script executed:
cat -n src/main/ipc/configValidation.ts | grep -A 3 -B 3 "isString"Repository: matt1398/claude-devtools
Length of output: 919
🏁 Script executed:
rg -n "isString" --type ts -A 2 -B 2 src/Repository: matt1398/claude-devtools
Length of output: 1125
🏁 Script executed:
rg -n "from.*stringUtils" src/renderer --type tsRepository: matt1398/claude-devtools
Length of output: 765
🏁 Script executed:
rg -n "import.*stringUtils" src/renderer --type tsRepository: matt1398/claude-devtools
Length of output: 765
Extract isString type guard to src/renderer/utils/stringUtils.ts instead of implementing it inline.
The inline typeof children === 'string' check violates the repo's type-guard convention. Add an isString helper to stringUtils.ts (which already exists and is used across renderer components) and import it via @renderer/utils/stringUtils:
Suggested adjustment
In src/renderer/utils/stringUtils.ts, add:
export const isString = (value: unknown): value is string => typeof value === 'string';Then in MarkdownViewer.tsx:
import { isString } from '@renderer/utils/stringUtils';
// Later in the code:
const raw = isString(children) ? children : '';This follows the established pattern where all string utilities are centralized in stringUtils.ts and imported by renderer components.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/chat/viewers/MarkdownViewer.tsx` around lines 157 -
161, Create and export an isString type guard in
src/renderer/utils/stringUtils.ts (export const isString = (value: unknown):
value is string => typeof value === 'string') and then update MarkdownViewer.tsx
to import it from '@renderer/utils/stringUtils' and replace the inline check in
the isBlock branch (where raw is computed from children) by using
isString(children) ? children : '' so the component uses the centralized string
utility instead of typeof checks.
There was a problem hiding this comment.
Code Review
The pull request introduces syntax highlighting for R, Ruby, PHP, and SQL in markdown code blocks, and wires this functionality into the MarkdownViewer. The changes also include the removal of some claude-sonnet-4-6 and claude-opus-4-6 entries from resources/pricing.json and the addition of a provider_specific_entry field to claude-opus-4-6 and claude-opus-4-6-20260205 models. The core changes related to syntax highlighting appear to be well-implemented, with appropriate keyword sets and comment detection logic. The integration into MarkdownViewer correctly applies the highlighting to code lines.
I am having trouble creating individual review comments. Click here to see my feedback.
resources/pricing.json (1636-1665)
The removal of the us/claude-sonnet-4-6 entry from the pricing data is a significant change. While the PR description focuses on syntax highlighting, this change in pricing.json could impact cost calculations or model availability. It's important to ensure this removal is intentional and doesn't inadvertently break any pricing logic or model selection in the application.
resources/pricing.json (1860-1951)
Similar to the previous comment, the removal of fast/claude-opus-4-6, us/claude-opus-4-6, and fast/us/claude-opus-4-6 entries from the pricing data is a substantial change. This could affect how these specific models are priced or even if they are available for use. Please confirm that these removals are intended and that there are no downstream dependencies that rely on these specific entries.
resources/pricing.json (1984-2044)
The removal of fast/claude-opus-4-6-20260205 and us/claude-opus-4-6-20260205 entries is another significant change. Please verify that these removals are intentional and that no other parts of the system depend on these specific pricing entries.
resources/pricing.json (1827-1831)
The addition of provider_specific_entry to claude-opus-4-6 is a new field. It's unclear from the PR description what this field represents or how it will be used. Please add a comment or update the PR description to explain the purpose of this new field and its expected values. This will improve maintainability and understanding for future developers.
resources/pricing.json (1863-1867)
The provider_specific_entry field is also added to claude-opus-4-6-20260205. As mentioned previously, please clarify the purpose and usage of this new field for better code understanding and maintainability.
Add keyword sets and comment detection for four new languages in the token-based syntax highlighter. Wire highlightLine into MarkdownViewer so fenced code blocks in assistant text also receive coloring. - R: keywords, # comments - Ruby: keywords, # comments - PHP: keywords, # comments - SQL: keywords: (case-insensitive), -- comments
61a31b6 to
022c75d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/chat/viewers/syntaxHighlighter.ts`:
- Line 256: The token "block_given?" is listed explicitly but the identifier
regex /^([a-zA-Z_$][a-zA-Z0-9_$]*)/ used by the tokenizer (in the
syntaxHighlighter module) does not accept trailing "?" or "!", so "block_given?"
will be split incorrectly; either remove the literal 'block_given?' entry from
the keyword/word list or update the identifier regex to accept Ruby-style method
suffixes (e.g. /^([a-zA-Z_$][a-zA-Z0-9_$]*[!?]?)/) so methods ending with ? or !
are captured as single identifiers (adjust any dependent tokenization logic
accordingly).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/renderer/components/chat/viewers/MarkdownViewer.tsxsrc/renderer/components/chat/viewers/syntaxHighlighter.ts
| 'raise', | ||
| 'return', | ||
| 'yield', | ||
| 'block_given?', |
There was a problem hiding this comment.
block_given? won't match as a keyword due to tokenizer regex.
The word-matching regex at line 535 (/^([a-zA-Z_$][a-zA-Z0-9_$]*)/) doesn't include ?, so block_given? will be tokenized as block_given (plain text) followed by ? (operator). Consider removing this entry or extending the regex to handle Ruby's ?/! method suffixes for this language.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/chat/viewers/syntaxHighlighter.ts` at line 256, The
token "block_given?" is listed explicitly but the identifier regex
/^([a-zA-Z_$][a-zA-Z0-9_$]*)/ used by the tokenizer (in the syntaxHighlighter
module) does not accept trailing "?" or "!", so "block_given?" will be split
incorrectly; either remove the literal 'block_given?' entry from the
keyword/word list or update the identifier regex to accept Ruby-style method
suffixes (e.g. /^([a-zA-Z_$][a-zA-Z0-9_$]*[!?]?)/) so methods ending with ? or !
are captured as single identifiers (adjust any dependent tokenization logic
accordingly).
Summary
highlightLineintoMarkdownViewerso fenced code blocks in assistant text also receive syntax coloring#for R/Ruby/PHP,--for SQLFiles Changed
src/renderer/components/chat/viewers/syntaxHighlighter.ts— keyword sets + comment/keyword matchingsrc/renderer/components/chat/viewers/MarkdownViewer.tsx— wire highlightLine into markdown code blocksTest plan
pnpm typecheckpassespnpm lint:fix— 0 errorspnpm test— 827 tests passSummary by CodeRabbit