Improve Tree-sitter language support#22
Conversation
(all the ones I want, at least!)
unsupported languages)
non-Tree-sitter-supported languages
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 19 minutes and 26 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughIntegrates Tree-sitter across the TUI: adds multi-language syntax highlighting, smart indentation/open-line logic, lexical fallback spans, and cache staleness tracking; expands language configs/styles (including Markdown) and tests; updates rendering call sites and buffer indentation helpers. Changes
Sequence DiagramsequenceDiagram
participant Editor as Editor (InputAction)
participant Buffer as TextBuffer
participant Syntax as SyntaxHighlighter
participant Language as LanguageConfig
participant Display as Display
Editor->>Buffer: get cursor position
Buffer-->>Editor: cursor_pos
Editor->>Syntax: language_for_path(buffer.path)
Syntax-->>Editor: language_option
Editor->>Syntax: smart_newline_insert(buffer, language, cursor)
Note over Syntax,Language: parse with Tree-sitter / combine queries → compute desired indent
Syntax->>Language: use grammar & highlight queries
Language-->>Syntax: scope/indent info
Syntax-->>Editor: (insert_text, new_cursor)?
alt smart indent returned
Editor->>Buffer: insert(insert_text) and set cursor
Buffer-->>Display: updated content
else fallback
Editor->>Buffer: insert_newline() and set fallback cursor
Buffer-->>Display: updated content
end
Editor->>Syntax: mark_cache_stale() (invalidate freshness, keep cached analysis)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/redox-tui/src/app/state/editing.rs (1)
614-619:⚠️ Potential issue | 🟠 MajorReindent happens before line relocation, causing stale column adjustment targets.
At Line 614 and Line 654,
reindent_active_line_spanruns before updatingvisual_anchor.line/cursor.line. Since column correction is line-matched, indent deltas can be skipped for moved selections.✅ Suggested fix (reorder operations)
@@ - self.reindent_active_line_span(new_start, new_end); let view = self.views.entry(active_id).or_default(); if let Some(anchor) = view.visual_anchor.as_mut() { anchor.line = anchor.line.saturating_sub(delta); } view.cursor.cursor.line = view.cursor.cursor.line.saturating_sub(delta); + self.reindent_active_line_span(new_start, new_end); let buffer = self.session.active_buffer(); view.cursor .reconcile_after_edit(buffer, viewport_width_cells, text_vh); @@ - self.reindent_active_line_span(new_start, new_end); let view = self.views.entry(active_id).or_default(); if let Some(anchor) = view.visual_anchor.as_mut() { anchor.line = anchor.line.saturating_add(delta); } view.cursor.cursor.line = view.cursor.cursor.line.saturating_add(delta); + self.reindent_active_line_span(new_start, new_end); let buffer = self.session.active_buffer(); view.cursor .reconcile_after_edit(buffer, viewport_width_cells, text_vh);Also applies to: 654-659
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/redox-tui/src/app/state/editing.rs` around lines 614 - 619, The reindent is being called before the selection's lines are relocated, so column-based corrections use stale line positions; move the call to reindent_active_line_span to after you update the view's positions. Specifically, for the blocks that fetch view via self.views.entry(active_id).or_default() and then adjust anchor.line (visual_anchor.as_mut() -> anchor.line = anchor.line.saturating_sub(delta)) and view.cursor.cursor.line = view.cursor.cursor.line.saturating_sub(delta), reorder so the visual_anchor and view.cursor.cursor.line updates happen first and then call reindent_active_line_span(new_start, new_end); apply the same reorder for the other occurrence that adjusts lines (the block around the second instance currently calling reindent before updating anchor/cursor).
🧹 Nitpick comments (1)
crates/redox-tui/src/ui/syntax/languages/json.rs (1)
7-17: Consider addingscope_kindsfor JSON smart indentation.The
scope_kindsarray is empty, which means smart auto-indentation won't recognize JSON objects/arrays as scopes. If smart indentation within JSON structures is desired, consider:- scope_kinds: &[], + scope_kinds: &["object", "array"],This is optional since JSON editing patterns may differ from code, but it would provide consistency with the PR's smart-indent goals.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/redox-tui/src/ui/syntax/languages/json.rs` around lines 7 - 17, The JSON_LANGUAGE config currently has an empty scope_kinds which prevents the smart indenter from treating JSON objects/arrays as block scopes; update the JSON_LANGUAGE.scope_kinds to include the JSON tree-sitter node kinds that should act as scopes (e.g. "object" and "array", and optionally "pair" if you want key/value pair-aware indentation) so the smart-indent logic can detect and indent inside JSON structures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/redox-tui/src/ui/overlays.rs`:
- Around line 66-84: The cache currently exposes stale analyses via get(), so
update the cache API and call sites to prevent rendering stale data: make get()
return only fresh analyses (use has_fresh_analysis()) or add a new get_fresh()
that returns Option<DelimiterAnalysis> only when has_fresh_analysis() is true;
modify callers (e.g., the render path that calls
view.delimiter_pair_cache.get()) to use the freshness-aware accessor; ensure
install(), mark_stale(), has_fresh_analysis(), and the test-only
has_stale_analysis() remain consistent with the new getter behavior so stale
analyses are never returned by the public get() used for rendering.
In `@crates/redox-tui/src/ui/syntax/languages/python.rs`:
- Around line 15-24: The scope_kinds array in python.rs currently lists only
parent statement nodes so clause blocks like else/except/finally resolve to the
parent; update the scope_kinds definition (the scope_kinds slice) to include the
Tree-sitter Python clause node kinds: "else_clause", "except_clause",
"except_group_clause", and "finally_clause" so each clause is treated as its own
scope for guides/indentation.
In `@crates/redox-tui/src/ui/syntax/mod.rs`:
- Around line 521-529: The virtual source rewrite currently replaces the text
after the cursor with `left`, producing `prefix + prefix`; instead, when the
cursor is mid-line you must remove the right-hand remainder so the parser sees
the line truncated at the cursor. In the block that builds `virtual_source`
(using `source`, `source_line_start_byte`, `left`, `line_text`, `cursor_byte`,
`line_end`), change the `source.replace_range(cursor_byte..line_end, &left)` to
remove the right side (e.g. `source.replace_range(cursor_byte..line_end, "")`)
so the virtual buffer contains the line split at the cursor rather than
duplicated prefix.
- Around line 1130-1145: The fallback comment detection in lexical_comment_end
treats any ';' or '#' anywhere as comment start, causing false positives (e.g.,
"foo(); bar();"). Modify the branch that matches b'#' | b';' so it only returns
Some(bytes.len()) when the character is at the start of the line or preceded by
whitespace/newline; otherwise return None. In other words, in
lexical_comment_end check the byte before cursor (or cursor==0) and only treat
'#' or ';' as a comment starter if cursor==0 ||
previous_byte.is_ascii_whitespace() || previous_byte == b'\n', keeping other
branches (//, --, /* */) unchanged and referencing the lexical_comment_end
function and the match arm that currently uses matches!(bytes[cursor], b'#' |
b';').
- Around line 669-686: opens_line() currently treats any trailing '>' as an
opening tag which makes closing tags (</div>) and self-closing tags (<img />)
indent the next line; change the HTML/TSX branch so it only returns true for
actual opening tags by ensuring the line text is not a closing tag (doesn't
start with "</" after trimming) and not a self-closing tag (doesn't end with
"/>" allowing for trailing whitespace) before calling opens_html_tag, or
alternatively update opens_html_tag to explicitly return false for
closing/self-closing tags; target the opens_line function and opens_html_tag
helper and keep existing checks using trailing_significant_char,
paired_closer_for, and node_kind_at_byte intact.
- Around line 393-404: The stale-scope fallback is currently reused for any
cursor in the same language; change the stale selection logic so it only returns
the cached scope when the cached entry matches the current cursor position. In
the block that computes stale_scope (using cache_stale, language, and
self.active_scope_cache), add a predicate that compares the cached entry's
cursor/location to the current cursor (and buffer if needed) before returning
cached.scope, so active_scope_pair_cached(buffer, language, analysis_version,
cursor).or(stale_scope) only uses stale_scope when the cached cursor equals the
current cursor.
In `@README.md`:
- Around line 200-202: The three checklist items "json", "toml", and "yaml" in
README.md are indented inconsistently causing markdownlint MD005; fix by
aligning their indentation to match the surrounding nested roadmap list (use the
same number of leading spaces or tabs as other nested items) so they appear as
properly nested bullets under their parent list entry; locate the lines
containing "json", "toml", and "yaml" and adjust their leading whitespace to the
same level as other nested items.
---
Outside diff comments:
In `@crates/redox-tui/src/app/state/editing.rs`:
- Around line 614-619: The reindent is being called before the selection's lines
are relocated, so column-based corrections use stale line positions; move the
call to reindent_active_line_span to after you update the view's positions.
Specifically, for the blocks that fetch view via
self.views.entry(active_id).or_default() and then adjust anchor.line
(visual_anchor.as_mut() -> anchor.line = anchor.line.saturating_sub(delta)) and
view.cursor.cursor.line = view.cursor.cursor.line.saturating_sub(delta), reorder
so the visual_anchor and view.cursor.cursor.line updates happen first and then
call reindent_active_line_span(new_start, new_end); apply the same reorder for
the other occurrence that adjusts lines (the block around the second instance
currently calling reindent before updating anchor/cursor).
---
Nitpick comments:
In `@crates/redox-tui/src/ui/syntax/languages/json.rs`:
- Around line 7-17: The JSON_LANGUAGE config currently has an empty scope_kinds
which prevents the smart indenter from treating JSON objects/arrays as block
scopes; update the JSON_LANGUAGE.scope_kinds to include the JSON tree-sitter
node kinds that should act as scopes (e.g. "object" and "array", and optionally
"pair" if you want key/value pair-aware indentation) so the smart-indent logic
can detect and indent inside JSON structures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7143b714-5101-4ea3-ad5b-ced2e8b5651a
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
README.mdcrates/redox-core/src/buffer/text_buffer/editing.rscrates/redox-tui/Cargo.tomlcrates/redox-tui/src/app/state.rscrates/redox-tui/src/app/state/actions.rscrates/redox-tui/src/app/state/commands.rscrates/redox-tui/src/app/state/editing.rscrates/redox-tui/src/app/state/tests.rscrates/redox-tui/src/lib.rscrates/redox-tui/src/ui/overlays.rscrates/redox-tui/src/ui/style.rscrates/redox-tui/src/ui/syntax/languages/c.rscrates/redox-tui/src/ui/syntax/languages/cpp.rscrates/redox-tui/src/ui/syntax/languages/css.rscrates/redox-tui/src/ui/syntax/languages/go.rscrates/redox-tui/src/ui/syntax/languages/html.rscrates/redox-tui/src/ui/syntax/languages/javascript.rscrates/redox-tui/src/ui/syntax/languages/json.rscrates/redox-tui/src/ui/syntax/languages/lua.rscrates/redox-tui/src/ui/syntax/languages/markdown.rscrates/redox-tui/src/ui/syntax/languages/mod.rscrates/redox-tui/src/ui/syntax/languages/python.rscrates/redox-tui/src/ui/syntax/languages/rust.rscrates/redox-tui/src/ui/syntax/languages/toml.rscrates/redox-tui/src/ui/syntax/languages/typescript.rscrates/redox-tui/src/ui/syntax/languages/yaml.rscrates/redox-tui/src/ui/syntax/mod.rs
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/redox-tui/src/app/state/tests.rs (1)
3702-3942: Smart-indent tests look coherent with the Tree-sitter indent logic.The new suite covers the main branches of
desired_indent_for_line_source/indent_after_line:
- Opening-delimiter Enter (Rust
{, angle in HTML, quotes, backticks in Markdown) → adds one tab level.- Splitting a paired closer on the same line → inserts indented blank line and pushes the closer down.
o/Obetween matching delimiters re-uses the smart indent.- Markdown list continuation / blank-line non-skip / partial-tab flooring behaviors match
floored_indent+markdown_indent_after_linesemantics.ccindent preservation andvisual movereindent against the new Tree-sitter scope.One minor thing worth double-checking:
insert_enter_splits_backtick_delimiters_with_smart_indent(Lines 3789-3806) relies on a bare`being treated as a paired delimiter bypaired_closer_for. Backticks are less conventional as a "pair" than{}/()/<>/"", so ifpaired_closer_foris ever tightened to only handle brackets/quotes, this test will regress silently from a "smart indent" test into an "unrelated newline" test. Worth a brief comment in the test noting the assumption, or pinning it via a direct unit test onpaired_closer_for('')`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/redox-tui/src/app/state/tests.rs` around lines 3702 - 3942, The backtick test relies on paired_closer_for treating '`' as a paired delimiter, which could silently regress; update the test insert_enter_splits_backtick_delimiters_with_smart_indent to include a short comment stating this assumption and add (or pin) a small direct unit test that asserts paired_closer_for('`') returns the expected closer so the contract is explicit and will fail-fast if paired_closer_for is tightened.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/redox-tui/src/app/state/tests.rs`:
- Around line 3702-3942: The backtick test relies on paired_closer_for treating
'`' as a paired delimiter, which could silently regress; update the test
insert_enter_splits_backtick_delimiters_with_smart_indent to include a short
comment stating this assumption and add (or pin) a small direct unit test that
asserts paired_closer_for('`') returns the expected closer so the contract is
explicit and will fail-fast if paired_closer_for is tightened.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 17c2c992-058c-42ac-94f9-7226ccec5de2
📒 Files selected for processing (6)
README.mdcrates/redox-tui/src/app/state/editing.rscrates/redox-tui/src/app/state/tests.rscrates/redox-tui/src/ui/overlays.rscrates/redox-tui/src/ui/syntax/languages/python.rscrates/redox-tui/src/ui/syntax/mod.rs
✅ Files skipped from review due to trivial changes (1)
- crates/redox-tui/src/ui/syntax/mod.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- README.md
- crates/redox-tui/src/app/state/editing.rs
- crates/redox-tui/src/ui/overlays.rs
- crates/redox-tui/src/ui/syntax/languages/python.rs
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/redox-tui/src/ui/syntax/mod.rs (1)
348-360:⚠️ Potential issue | 🟡 MinorCheck
cache_stalebefore returning the active-scope cache.Line 350 can return a cached scope even after
mark_cache_stale()if the caller passes the sameanalysis_versionand cursor. That bypasses the stale guard on Line 358 and makes the non-display API expose stale data.Proposed fix
let language = language?; let cursor_char = buffer.pos_to_char(cursor); + if self.cache_stale { + return None; + } + if let Some(cached) = self.active_scope_cache && cached.language == language && cached.analysis_version == analysis_version && cached.cursor_char == cursor_char { return cached.scope; } - if self.cache_stale { - return None; - } - let config = language_config_for(language)?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/redox-tui/src/ui/syntax/mod.rs` around lines 348 - 360, The code currently returns self.active_scope_cache even when self.cache_stale is true if the cached entry matches language/analysis_version/cursor; change the logic so self.cache_stale is checked before returning cached data—either move the "if self.cache_stale { return None; }" above the active-scope cache branch or include "&& !self.cache_stale" in the cached-match condition; ensure you reference self.active_scope_cache, cached.analysis_version, cached.cursor_char, and self.cache_stale when making the check so stale caches are never returned (also keep the language = language? and cursor_char lookup intact).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/redox-tui/src/ui/syntax/mod.rs`:
- Around line 671-677: The opens_line function currently treats any character
with a paired closer (via paired_closer_for) as opening a block, which wrongly
treats inline backticks as block openers; update opens_line (and the same logic
around the other occurrence at the paired-closing check between lines 801–807)
to explicitly ignore the backtick character: after obtaining ch from
trailing_significant_char, return false if ch == '`' before calling
paired_closer_for, so inline Markdown code (e.g., `code`) will not trigger block
indentation.
---
Outside diff comments:
In `@crates/redox-tui/src/ui/syntax/mod.rs`:
- Around line 348-360: The code currently returns self.active_scope_cache even
when self.cache_stale is true if the cached entry matches
language/analysis_version/cursor; change the logic so self.cache_stale is
checked before returning cached data—either move the "if self.cache_stale {
return None; }" above the active-scope cache branch or include "&&
!self.cache_stale" in the cached-match condition; ensure you reference
self.active_scope_cache, cached.analysis_version, cached.cursor_char, and
self.cache_stale when making the check so stale caches are never returned (also
keep the language = language? and cursor_char lookup intact).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6fc9e847-34c0-4e2b-8259-e9283a7ffd85
📒 Files selected for processing (2)
crates/redox-tui/src/app/state/tests.rscrates/redox-tui/src/ui/syntax/mod.rs
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation