Add bare filename regex for link detection#17
Add bare filename regex for link detection#17arigoldx wants to merge 8 commits intomanaflow-ai:mainfrom
Conversation
Adds a new branch to the URL regex that matches bare filenames with common source code extensions (tsx, py, json, etc.). Uses negative lookbehind (?<![/\w]) to match filenames not preceded by slash or word char, enabling detection in ls column output.
📝 WalkthroughWalkthroughAdded a new build step for the CLI helper and enhanced theme preview functionality with cmux configuration support. Introduced live preview with environment-based overrides, improved terminal output buffering, and extended URL parsing to detect standalone filenames with extensions. Changes
Sequence DiagramsequenceDiagram
participant TUI as Theme Picker TUI
participant Env as Environment
participant FS as Filesystem
participant Darwin as Darwin Notifications
TUI->>Env: Check for CMUX_THEME_PICKER_CONFIG
alt Cmux mode enabled
TUI->>FS: Read config file from env path
FS-->>TUI: Config contents
TUI->>TUI: Load cmux state (light/dark themes)
Note over TUI: Display preview with current theme
TUI->>TUI: User navigates/edits (up/down/search/filter)
TUI->>TUI: Apply theme override to cmux state
alt User presses Enter
TUI->>FS: Write updated config with override block
TUI->>Darwin: Post com.cmuxterm.themes.reload-config
Note over TUI: Exit with apply outcome
else User presses Escape
TUI->>FS: Restore original config (remove override)
Darwin->>TUI: Reload notification (implicit)
Note over TUI: Exit with cancel outcome
end
else Cmux mode disabled
Note over TUI: Display traditional save flow
end
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🧪 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 Tip Migrating from UI to YAML configuration.Use the |
Greptile SummaryThis PR bundles two largely independent sets of changes under a single title: (1) a new Branch 4 to the URL/path regex that detects bare filenames with known source-code extensions (the stated purpose), and (2) a substantial cmux terminal integration into the theme-picker TUI that enables live-preview and config-file management for the external cmux terminal app. The buffered-writer update in Key observations:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Terminal text line] --> B{Branch 1:\nscheme URL?}
B -->|yes| Z[Highlight match]
B -->|no| C{Branch 2:\nrooted/relative path?}
C -->|yes| Z
C -->|no| D{Branch 3:\nbare relative path\ne.g. src/foo.zig}
D -->|yes| Z
D -->|no| E{Branch 4 NEW:\nbare filename\nwith known extension}
E -->|yes| Z
E -->|no| F[No match]
subgraph Branch4 [Branch 4 logic]
E1["(?<![/\\w]) — not preceded\nby slash or word char"]
E2["[\\w][\\w\\-.]*\\. — word chars\nfollowed by dot"]
E3["EXTENSION — known ext list\ne.g. py, ts, go, zig ..."]
E4["(?![:\\w]) — not followed\nby colon or word char"]
E1 --> E2 --> E3 --> E4
end
E --> Branch4
|
| fn initialTheme(self: *const CmuxThemePicker) ?[]const u8 { | ||
| return switch (self.target_mode) { | ||
| .both => if (eqlOptionalTheme(self.initial_light, self.initial_dark)) | ||
| self.initial_light orelse self.initial_dark | ||
| else | ||
| self.initial_dark orelse self.initial_light, | ||
| .light => self.initial_light orelse self.initial_dark, | ||
| .dark => self.initial_dark orelse self.initial_light, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Dead code:
initialTheme is never called
CmuxThemePicker.initialTheme is defined here but is never invoked anywhere in the codebase. A search across all of src/ confirms the only occurrence of initialTheme is this definition. If this method is intended for a future feature, it should be documented with a // TODO comment; otherwise it should be removed to avoid confusion and to keep the dead-code surface small.
| const bare_filename_with_extension = | ||
| \\(?<![/\w])[\w][\w\-.]*\. | ||
| ++ common_extensions ++ | ||
| \\(?![:\w]) | ||
| ; |
There was a problem hiding this comment.
Branch 4 won't match filenames followed by
:line numbers
The negative lookahead (?![:\w]) prevents matching when a : immediately follows the extension. This means file.py:10 and file.py:10:5 (typical compiler/linter error output formats) will not be detected as links by this branch.
Branch 3 (bare relative paths) handles this correctly — e.g. app/folder/file.rb:1 matches with the :1 included because path_chars contains :. For bare filenames without a directory component, there is no fallback. Consider whether the lookahead should be relaxed to permit an optional :digits suffix, e.g.:
(?![:\w])
→ replaced with something that still blocks file.pyx (word char) but allows file.py:10:
(?![\w])
or handle a (:[\d]+)* optional suffix inside the match. As written, a bare error.py:42 from a compiler won't be clickable.
| const bare_filename_with_extension = | ||
| \\(?<![/\w])[\w][\w\-.]*\. | ||
| ++ common_extensions ++ | ||
| \\(?![:\w]) | ||
| ; |
There was a problem hiding this comment.
Lookbehind allows matches after
. — potential false positives
(?<![/\w]) only prevents a match when preceded by / or a word character ([A-Za-z0-9_]). A dot (.) is not excluded. This means version.file.py would produce a match of file.py starting at the f (the preceding . satisfies the lookbehind). Similarly v1.main.go would match main.go.
While this may be acceptable in practice (such strings rarely appear in terminal output without a preceding path separator), it is worth being aware of when triaging false-positive reports.
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)
src/cli/list_themes.zig (1)
871-882:⚠️ Potential issue | 🟡 MinorAdd bounds check before clipboard operations.
The clipboard copy operations access
self.filtered.items[self.current]without verifying the list is non-empty, which could cause an out-of-bounds access.🐛 Proposed fix
- if (key.matches('c', .{})) + if (key.matches('c', .{}) and self.filtered.items.len > 0) try self.vx.copyToSystemClipboard( self.tty.writer(), self.themes[self.filtered.items[self.current]].theme, alloc, ) - else if (key.matches('c', .{ .shift = true })) + else if (key.matches('c', .{ .shift = true }) and self.filtered.items.len > 0) try self.vx.copyToSystemClipboard( self.tty.writer(), self.themes[self.filtered.items[self.current]].path, alloc, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/list_themes.zig` around lines 871 - 882, Before calling self.vx.copyToSystemClipboard, ensure the index is valid by checking self.filtered.items.len > 0 and that self.current < self.filtered.items.len; if the check fails, skip the clipboard call (or return/emit an error) to avoid out‑of‑bounds access when reading self.filtered.items[self.current] and then indexing into self.themes; apply this guard in both branches where self.vx.copyToSystemClipboard is invoked (the branch copying .theme and the branch copying .path) and reuse the same validation to keep behavior consistent.
🧹 Nitpick comments (2)
src/config/url.zig (2)
121-123: Consider case-insensitive extension matching.The extension list is case-sensitive, so
README.MD,Config.JSON, orMakefile.PYwon't match. While lowercase extensions are more common, some systems or users may use uppercase.If case-insensitive matching is desired, the Oniguruma
(?i:...)syntax could be used for the extension part. However, this may be an intentional design choice to avoid false positives.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config/url.zig` around lines 121 - 123, The current regex constant common_extensions only lists lowercase extensions so it won't match uppercase extensions; update common_extensions to perform case-insensitive matching for the extension group (e.g., wrap the alternation in an Oniguruma case-insensitive group like (?i:... ) or add the inline (?i) flag scoped to the extension part) or alternatively normalize the input filename to lowercase before matching; locate and modify the common_extensions definition to apply the chosen fix.
116-137: Add test cases for the new bare filename regex branch.The new
bare_filename_with_extensionbranch looks correct for matching standalone filenames inlsandgit statusoutput. However, no test cases were added to verify its behavior.Consider adding test cases to
test "url regex"for:
- Positive matches:
"main.zig","README.md","config.json"- Negative matches (should not match): filenames preceded by
/or word chars like"path/file.py"(should match the path branch instead),"wordfile.txt"(preceded by word char)This would ensure the lookbehind/lookahead assertions work as expected.
📝 Suggested test cases
.input = "./Downloads: Operation not permitted", .expect = "./Downloads", }, + // Bare filenames with common extensions (Branch 4) + .{ + .input = "main.zig", + .expect = "main.zig", + }, + .{ + .input = "README.md is the documentation", + .expect = "README.md", + }, + .{ + .input = "check config.json for settings", + .expect = "config.json", + }, + .{ + .input = "file.tsx another.py third.rs", + .expect = "file.tsx", + }, };And for no-match cases, verify that filenames preceded by word characters don't match incorrectly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config/url.zig` around lines 116 - 137, Add unit tests to the existing test "url regex" that exercise the new bare_filename_with_extension branch and the combined regex: assert positive matches for standalone filenames like "main.zig", "README.md", and "config.json" (ensure they are matched by regex) and assert negative/no-match or alternative-branch behavior for inputs that should not hit this branch such as paths like "path/file.py" (should be matched by the path branch instead) and tokens preceded by word characters like "wordfile.txt" (should not be matched as a bare filename); locate tests using the regex symbol named regex and the branch symbol bare_filename_with_extension to verify lookbehind/lookahead behavior.
🤖 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/cli/list_themes.zig`:
- Around line 839-842: The End key handler can underflow when
self.filtered.items.len == 0 because it sets self.current =
self.filtered.items.len - 1 without checking; update the branch that checks
key.matchesAny(&.{ vaxis.Key.end, vaxis.Key.kp_end }, .{}) to first guard that
self.filtered.items.len > 0 and only then set self.current =
self.filtered.items.len - 1 and call try
self.applyCmuxSelectionForCurrentTheme(); keep the rest of the End key logic
unchanged and use the same symbols (self.current, self.filtered.items.len,
applyCmuxSelectionForCurrentTheme, key.matchesAny) so behavior matches
up()/down() handling of empty lists.
---
Outside diff comments:
In `@src/cli/list_themes.zig`:
- Around line 871-882: Before calling self.vx.copyToSystemClipboard, ensure the
index is valid by checking self.filtered.items.len > 0 and that self.current <
self.filtered.items.len; if the check fails, skip the clipboard call (or
return/emit an error) to avoid out‑of‑bounds access when reading
self.filtered.items[self.current] and then indexing into self.themes; apply this
guard in both branches where self.vx.copyToSystemClipboard is invoked (the
branch copying .theme and the branch copying .path) and reuse the same
validation to keep behavior consistent.
---
Nitpick comments:
In `@src/config/url.zig`:
- Around line 121-123: The current regex constant common_extensions only lists
lowercase extensions so it won't match uppercase extensions; update
common_extensions to perform case-insensitive matching for the extension group
(e.g., wrap the alternation in an Oniguruma case-insensitive group like (?i:...
) or add the inline (?i) flag scoped to the extension part) or alternatively
normalize the input filename to lowercase before matching; locate and modify the
common_extensions definition to apply the chosen fix.
- Around line 116-137: Add unit tests to the existing test "url regex" that
exercise the new bare_filename_with_extension branch and the combined regex:
assert positive matches for standalone filenames like "main.zig", "README.md",
and "config.json" (ensure they are matched by regex) and assert
negative/no-match or alternative-branch behavior for inputs that should not hit
this branch such as paths like "path/file.py" (should be matched by the path
branch instead) and tokens preceded by word characters like "wordfile.txt"
(should not be matched as a bare filename); locate tests using the regex symbol
named regex and the branch symbol bare_filename_with_extension to verify
lookbehind/lookahead behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: caa545e4-7830-4486-bd04-43db32802989
📒 Files selected for processing (4)
build.zigsrc/cli/list_themes.zigsrc/config/url.zigsrc/main_ghostty.zig
| if (key.matchesAny(&.{ vaxis.Key.end, vaxis.Key.kp_end }, .{})) { | ||
| self.current = self.filtered.items.len - 1; | ||
| if (key.matchesAny(&.{ 'j', '+', vaxis.Key.down, vaxis.Key.kp_down, vaxis.Key.kp_add }, .{})) | ||
| try self.applyCmuxSelectionForCurrentTheme(); | ||
| } |
There was a problem hiding this comment.
Potential underflow when pressing End key with empty filtered list.
If self.filtered.items.len == 0, the expression self.filtered.items.len - 1 will underflow (wrap to maxInt(usize)). The up() and down() methods handle empty lists, but the End key handler does not.
🐛 Proposed fix
if (key.matchesAny(&.{ vaxis.Key.end, vaxis.Key.kp_end }, .{})) {
- self.current = self.filtered.items.len - 1;
+ if (self.filtered.items.len > 0) {
+ self.current = self.filtered.items.len - 1;
+ }
try self.applyCmuxSelectionForCurrentTheme();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (key.matchesAny(&.{ vaxis.Key.end, vaxis.Key.kp_end }, .{})) { | |
| self.current = self.filtered.items.len - 1; | |
| if (key.matchesAny(&.{ 'j', '+', vaxis.Key.down, vaxis.Key.kp_down, vaxis.Key.kp_add }, .{})) | |
| try self.applyCmuxSelectionForCurrentTheme(); | |
| } | |
| if (key.matchesAny(&.{ vaxis.Key.end, vaxis.Key.kp_end }, .{})) { | |
| if (self.filtered.items.len > 0) { | |
| self.current = self.filtered.items.len - 1; | |
| } | |
| try self.applyCmuxSelectionForCurrentTheme(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli/list_themes.zig` around lines 839 - 842, The End key handler can
underflow when self.filtered.items.len == 0 because it sets self.current =
self.filtered.items.len - 1 without checking; update the branch that checks
key.matchesAny(&.{ vaxis.Key.end, vaxis.Key.kp_end }, .{}) to first guard that
self.filtered.items.len > 0 and only then set self.current =
self.filtered.items.len - 1 and call try
self.applyCmuxSelectionForCurrentTheme(); keep the rest of the End key logic
unchanged and use the same symbols (self.current, self.filtered.items.len,
applyCmuxSelectionForCurrentTheme, key.matchesAny) so behavior matches
up()/down() handling of empty lists.
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/config/url.zig">
<violation number="1" location="src/config/url.zig:127">
P2: The trailing lookahead currently blocks `:` so bare compiler-style references like `error.py:42` are not linkified. Allow an optional `:line[:column]` suffix while still rejecting trailing word characters.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const bare_filename_with_extension = | ||
| \\(?<![/\w])[\w][\w\-.]*\. | ||
| ++ common_extensions ++ | ||
| \\(?![:\w]) |
There was a problem hiding this comment.
P2: The trailing lookahead currently blocks : so bare compiler-style references like error.py:42 are not linkified. Allow an optional :line[:column] suffix while still rejecting trailing word characters.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/config/url.zig, line 127:
<comment>The trailing lookahead currently blocks `:` so bare compiler-style references like `error.py:42` are not linkified. Allow an optional `:line[:column]` suffix while still rejecting trailing word characters.</comment>
<file context>
@@ -113,12 +113,28 @@ const bare_relative_path_branch =
+const bare_filename_with_extension =
+ \\(?<![/\w])[\w][\w\-.]*\.
+++ common_extensions ++
+ \\(?![:\w])
+;
+
</file context>
| \\(?![:\w]) | |
| \\(?::\d+(?::\d+)?)?(?![:\w]) |
Brief Summary
I wanted the functionality from iTerm where you can command-click on a filename and it will open in your favorite editor.
Thorough Summary
(?<![/\w])to match filenames not preceded by slash or word charlscolumn output andgit statusTest plan
lsoutput - all columns underlinegit status- bare filenames underlineSummary by cubic
Adds bare filename link detection and a cmux-integrated theme picker with live preview and apply/cancel. Improves build steps and CLI output reliability.
lscolumns andgit status.cli-helperbuild step; buffered and flushed usage output inapp_runtime=none.Written for commit 8a9cbe9. Summary will update on new commits.
Summary by CodeRabbit
New Features
Build