Analyzed and summarized GitHub PR #907
Public
Analyzed and summarized GitHub PR #907
Agent
Pi
Model
Claude Opus 4.5 (latest)
Started
Jan 22, 9:45 PM
Messages132
Downloads0
Reviewed linked issue and code files.
UnderstandChecked related documentation for updates.
UnderstandSummarized code changes and tests.
Verify
You are given one or more GitHub PR URLs: https://github.com/badlogic/pi-mono/pull/907
For each PR URL, do the following in order:
packages/*/CHANGELOG.md files. Report whether an entry exists. If missing, state that a changelog entry is required before merge and that you will add it if the user decides to merge. Follow the changelog format rules in AGENTS.md. Verify:
### Breaking Changes, ### Added, ### Fixed, etc.)Fixed foo ([#123](https://github.com/badlogic/pi-mono/pull/123) by [@user](https://github.com/user))### Breaking Changes, not just ### FixedOutput format per PR: PR: Changelog:
If no issues are found, say so under Bad and Ugly.
The user wants me to review a GitHub PR. Let me start by fetching the PR details including description, comments, commits, and changed files.
>gh pr view 907 --repo badlogic/pi-mono --json title,body,comments,commits,files,author,labels,state,baseRefName,headRefName{"author":{"id":"MDQ6VXNlcjYzMjY0NDA=","is_bot":false,"login":"tmustier","name":"Thomas Mustier"},"baseRefName":"main","body":"Per https://github.com/badlogic/pi-mono/issues/904\r\n- Change: adds a helper (`isSlashMenuAlllowed`) in `packages/tui/src/components/editor.ts` to detect nonāwhitespace anywhere outside the cursor position.\r\n- Impact: only triggers slash-command autocomplete when that helper returns false; otherwise inserts / as normal text.\r\n- Testing: \r\n - npm run check\r\n - ./test.sh\r\n - manual testing locally\r\n\r\nBuilt with Pi:\r\n- Share URL: https://buildwithpi.ai/session/#8e831bd60b2078d5b7d9174bf83d21df \r\n- Gist: https://gist.github.com/tmustier/8e831bd60b2078d5b7d9174bf83d21df ","comments":[],"commits":[{"authoredDate":"2026-01-22T12:46:29Z","authors":[{"email":"[REDACTED]","id":"MDQ6VXNlcjYzMjY0NDA=","login":"tmustier","name":"Thomas Mustier"}],"committedDate":"2026-01-22T12:46:29Z","messageBody":"","messageHeadline":"fix(tui): gate slash menu to empty input","oid":"8c895739e797e301b6b7e8ac30ffa66a38f63b76"}],"files":[{"path":"packages/tui/src/components/editor.ts","additions":35,"deletions":5}],"headRefName":"fix/slash-menu-trigger","labels":[],"state":"OPEN","title":"fix(tui): gate slash menu to empty input"}>gh pr diff 907 --repo badlogic/pi-monodiff --git a/packages/tui/src/components/editor.ts b/packages/tui/src/components/editor.ts
index ed1739377..a627e3d37 100644
--- a/packages/tui/src/components/editor.ts
+++ b/packages/tui/src/components/editor.ts
@@ -984,7 +984,7 @@ export class Editor implements Component, Focusable {
const currentLine = this.state.lines[this.state.cursorLine] || "";
const textBeforeCursor = currentLine.slice(0, this.state.cursorCol);
// Check if we're in a slash command (with or without space for arguments)
- if (textBeforeCursor.trimStart().startsWith("/")) {
+ if (this.isInSlashCommandContext(textBeforeCursor)) {
this.tryTriggerAutocomplete();
}
// Check if we're in an @ file reference context
@@ -1169,7 +1169,7 @@ export class Editor implements Component, Focusable {
const currentLine = this.state.lines[this.state.cursorLine] || "";
const textBeforeCursor = currentLine.slice(0, this.state.cursorCol);
// Slash command context
- if (textBeforeCursor.trimStart().startsWith("/")) {
+ if (this.isInSlashCommandContext(textBeforeCursor)) {
this.tryTriggerAutocomplete();
}
// @ file reference context
@@ -1388,7 +1388,7 @@ export class Editor implements Component, Focusable {
const currentLine = this.state.lines[this.state.cursorLine] || "";
const textBeforeCursor = currentLine.slice(0, this.state.cursorCol);
// Slash command context
- if (textBeforeCursor.trimStart().startsWith("/")) {
+ if (this.isInSlashCommandContext(textBeforeCursor)) {
this.tryTriggerAutocomplete();
}
// @ file reference context
@@ -1802,13 +1802,43 @@ export class Editor implements Component, Focusable {
}
}
+ private isSlashMenuAllowed(): boolean {
+ const currentLine = this.state.lines[this.state.cursorLine] ?? "";
+ const afterCursor = currentLine.slice(this.state.cursorCol);
+
+ if (afterCursor.trim() !== "") {
+ return false;
+ }
+
+ for (let i = 0; i < this.state.lines.length; i++) {
+ if (i === this.state.cursorLine) continue;
+ if ((this.state.lines[i] ?? "").trim() !== "") {
+ return false;
+ }
+ }
+ return true;
+ }
+
// Helper method to check if cursor is at start of message (for slash command detection)
private isAtStartOfMessage(): boolean {
+ if (!this.isSlashMenuAllowed()) {
+ return false;
+ }
+
const currentLine = this.state.lines[this.state.cursorLine] || "";
const beforeCursor = currentLine.slice(0, this.state.cursorCol);
+ const trimmedBeforeCursor = beforeCursor.trim();
// At start if line is empty, only contains whitespace, or is just "/"
- return beforeCursor.trim() === "" || beforeCursor.trim() === "/";
+ return trimmedBeforeCursor === "" || trimmedBeforeCursor === "/";
+ }
+
+ private isInSlashCommandContext(textBeforeCursor: string): boolean {
+ if (!this.isSlashMenuAllowed()) {
+ return false;
+ }
+
+ return textBeforeCursor.trimStart().startsWith("/");
}
// Autocomplete methods
@@ -1848,7 +1878,7 @@ export class Editor implements Component, Focusable {
const beforeCursor = currentLine.slice(0, this.state.cursorCol);
// Check if we're in a slash command context
- if (beforeCursor.trimStart().startsWith("/") && !beforeCursor.trimStart().includes(" ")) {
+ if (this.isInSlashCommandContext(beforeCursor) && !beforeCursor.trimStart().includes(" ")) {
this.handleSlashCommandCompletion();
} else {
this.forceFileAutocomplete();Now let me read the linked issue #904 to understand the context.
Let me read the linked issue #904 and the full editor.ts file:
>gh issue view 904 --repo badlogic/pi-mono --json title,body,comments,labels,state{"body":"### What do you want to change?\n\nHi there! This is a bug, but I am going to submit a PR for it this afternoon so I added here - let me know if wrong place!\n\nWhat I want to change: adjust slash command autocomplete in the TUI editor so / only opens the command menu when the input is otherwise empty (no non-whitespace anywhere else in the message). If the user has already typed text on any line, / should be treated as a normal character.\n\n### Why?\n\nCurrently, / at the start of any newline opens the menu even after other text has been entered; the menu isn't functional (selecting a command just pastes it as text and sends a message). The menu should only open when / is the first thing inputted\n<img width=\"718\" height=\"240\" alt=\"Image\" src=\"https://github.com/user-attachments/assets/8132882e-c916-4aa9-941a-f368141bdaf0\" />\n\n### How? (optional)\n\nGate slash-command autocomplete on āeditor is otherwise empty,ā i.e., no nonāwhitespace in any other line or after the cursor. Keep existing behavior for empty input; treat / as normal text when any other content exists:\n- Add a helper in packages/tui/src/components/editor.ts to detect nonāwhitespace anywhere outside the cursor position. \n - Only trigger slash-command autocomplete when that helper returns false; otherwise insert / as normal text. ","comments":[],"labels":[],"state":"OPEN","title":"Fix slash command menu triggering on non-empty multi-line input"}Let me continue reading the editor.ts file to see the rest of the changes, especially the isAtStartOfMessage method and other related functions.
Let me read the rest of the editor.ts file to see the isAtStartOfMessage and isInSlashCommandContext methods:
I've now read:
Let me now check if there's documentation that might need updating in coding-agent:
Now let me check if any coding-agent documentation needs updating:
>ls -la packages/coding-agent/docs/total 400
drwxr-xr-x@ 11 badlogic staff 352 Jan 22 13:52 .
drwxr-xr-x@ 18 badlogic staff 576 Jan 22 13:52 ..
-rw-r--r--@ 1 badlogic staff 14757 Jan 3 00:47 compaction.md
-rw-r--r--@ 1 badlogic staff 55153 Jan 22 13:52 extensions.md
-rw-r--r--@ 1 badlogic staff 23979 Jan 16 23:14 rpc.md
-rw-r--r--@ 1 badlogic staff 27799 Jan 22 13:52 sdk.md
-rw-r--r--@ 1 badlogic staff 10015 Jan 19 00:18 session.md
-rw-r--r--@ 1 badlogic staff 7936 Jan 22 13:52 skills.md
-rw-r--r--@ 1 badlogic staff 16734 Jan 8 19:37 theme.md
-rw-r--r--@ 1 badlogic staff 6645 Jan 16 21:49 tree.md
-rw-r--r--@ 1 badlogic staff 26947 Jan 19 15:55 tui.md>grep -r "slash" packages/coding-agent/README.md packages/coding-agent/docs/ 2>/dev/null || echo "No matches"packages/coding-agent/README.md: - [Slash Commands](#slash-commands)
packages/coding-agent/docs/skills.md:Skills are automatically registered as slash commands with a `/skill:` prefix:The slash command documentation mentions skills, but this is about the TUI editor behavior for when the slash menu triggers. This is an internal behavior fix, not a user-facing feature change that would require documentation updates. The documentation describes what slash commands do, not how the autocomplete triggers.
Let me verify the fix by understanding the original issue and the implementation:
Original Issue (#904):
/ at the start of any newline, even after other text was entered/ is the first thing inputted (when the editor is otherwise empty)The Fix:
Added isSlashMenuAllowed() - checks if the editor is "empty enough" for slash commands:
false if there's non-whitespace after the cursor on the current linefalse if there's non-whitespace on any other lineModified isAtStartOfMessage() to call isSlashMenuAllowed() first
Added isInSlashCommandContext() which also calls isSlashMenuAllowed() first
Updated 4 places that were checking textBeforeCursor.trimStart().startsWith("/") to use isInSlashCommandContext() instead
Analysis:
Good:
Potential concerns:
In isSlashMenuAllowed(), the check for "other lines" uses a strict iteration. This is correct.
The isAtStartOfMessage() changes now include isSlashMenuAllowed() check, plus a variable rename from beforeCursor.trim() to trimmedBeforeCursor with local variable. This is a minor refactor but the logic is equivalent.
The new isInSlashCommandContext() method properly enforces the empty-editor requirement. The four locations where the old check was replacedāin insertCharacter(), handleBackspace(), handleForwardDelete(), and handleTabCompletion()ānow all consistently use this gated approach instead of just checking for the slash prefix.
I notice the PR description has a typo in the method name ("isSlashMenuAlllowed" with three l's), though the actual implementation is correct. The changelog also appears to be missing an entry documenting this fix. I should check whether there are existing tests covering this behavior that might need updates.
Let me check for any existing tests related to slash command behavior:
>find packages/tui -name "*.test.ts" -exec grep -l -i "slash\|autocomplete\|StartOfMessage" {} \; 2>/dev/nullpackages/tui/test/editor.test.ts
packages/tui/test/input.test.ts
packages/tui/test/autocomplete.test.ts>ls packages/tui/test/ 2>/dev/null || echo "No test directory"autocomplete.test.ts
chat-simple.ts
editor.test.ts
fuzzy.test.ts
image-test.ts
input.test.ts
key-tester.ts
keys.test.ts
markdown.test.ts
overlay-options.test.ts
overlay-short-content.test.ts
select-list.test.ts
stdin-buffer.test.ts
test-themes.ts
truncated-text.test.ts
tui-overlay-style-leak.test.ts
tui-render.test.ts
virtual-terminal.ts
wrap-ansi.test.tsLet me look at the autocomplete test to see if it tests slash command triggering behavior.
This test file tests autocomplete for file suggestions and paths, but doesn't directly test the slash menu triggering behavior for slash commands. However, the test "does not trigger for slash commands" at line 71-81 checks that force file suggestions don't trigger for slash commands, which is related but different.
Let me check the editor.test.ts for any tests related to slash commands or autocomplete triggering.
>grep -n "slash\|autocomplete\|StartOfMessage\|isSlash\|isIn" packages/tui/test/editor.test.ts | head -504:import type { AutocompleteProvider } from "../src/autocomplete.js";
296: it("inserts a literal backslash when not followed by Enter", () => {
1531: it("undoes autocomplete", () => {
1534: // Create a mock autocomplete provider
1568: // Press Tab to trigger autocompleteThe tests don't specifically test when the slash menu should trigger. The fix is a behavior change that could benefit from a test, but the existing tests don't cover this specific edge case.
Let me now summarize my review.
PR: https://github.com/badlogic/pi-mono/pull/907
Changelog:
packages/tui/CHANGELOG.md has no entry for this fix under ## [Unreleased]. A changelog entry is required before merge. I will add it if you decide to merge.Good:
/ appeared at the start of any line in multi-line input, even when other content existedisSlashMenuAllowed() helper that checks all lines for non-whitespace contentisInSlashCommandContext() encapsulates the gating logic, replacing 4 instances of inline checksBad:
/ on a new line after entering text) could be covered by a unit test to prevent regressionUgly:
Questions or Assumptions:
isSlashMenuAllowed() check iterates over all lines on every keystroke in slash command context. For typical editor usage (< 100 lines), this is negligible. If large pastes become common, this could be optimized, but it's not a concern for current usage patterns.Change summary:
isSlashMenuAllowed() method (lines 1805-1819): Returns false if there's non-whitespace after cursor on current line, or non-whitespace on any other lineisAtStartOfMessage() (lines 1822-1832): Now calls isSlashMenuAllowed() first before checking cursor positionisInSlashCommandContext() method (lines 1834-1840): Combines isSlashMenuAllowed() check with the existing / prefix detectiontextBeforeCursor.trimStart().startsWith("/") to use the new gated isInSlashCommandContext():
insertCharacter() for typing alphanumericshandleBackspace() for re-triggering after backspacehandleForwardDelete() for re-triggering after deletehandleTabCompletion() for Tab-triggered completionTests:
npm run check, ./test.sh, and manual testingwhat inputs would that fix? line \n\n/?
The user is asking what specific input scenarios the fix addresses. Let me think through the logic of isSlashMenuAllowed():
private isSlashMenuAllowed(): boolean { const currentLine = this.state.lines[this.state.cursorLine] ?? ""; const afterCursor = currentLine.slice(this.state.cursorCol);
// Check 1: non-whitespace AFTER cursor on current line if (afterCursor.trim() !== "") { return false; }
// Check 2: non-whitespace on ANY OTHER line for (let i = 0; i < this.state.lines.length; i++) { if (i === this.state.cursorLine) continue; if ((this.state.lines[i] ?? "").trim() !== "") { return false; } } return true;}So for the user's example: line \n\n/
That's 3 lines:
When typing / on line 2:
afterCursor = "" (nothing after cursor) ā passes check 1falseSo yes, line\n\n/ would be blocked by this fix. The slash menu would NOT appear because line 0 has content.
What inputs WOULD still allow the slash menu?
/ (single line, only slash) / (whitespace before slash)\n/ (empty line, then slash on new line) \n \n/ (only whitespace on all lines before the slash)What inputs are now BLOCKED that weren't before?
Scenarios like text\n/ where there's content on an earlier line, or /model something\n/ where a command with arguments exists and the user tries to start another slash command on a new line. Essentially, any case where non-whitespace appears anywhere outside the current line gets rejected.
Yes, exactly. With the fix:
line\n\n/ (or line \n\n/) ā slash menu is blocked
The isSlashMenuAllowed() check iterates all lines and returns false if any line (other than the cursor line) contains non-whitespace:
for (let i = 0; i < this.state.lines.length; i++) { if (i === this.state.cursorLine) continue; if ((this.state.lines[i] ?? "").trim() !== "") { return false; // "line" on line 0 triggers this }}Scenarios now blocked:
hello\n/ ā text on line 0, slash on line 1\nhello\n/ ā text on line 1, slash on line 2/model gpt-4\n/ ā after completing one command, starting anotherScenarios still allowed:
/ ā empty editor, just typed slash / ā only whitespace before slash\n\n/ ā all other lines are empty/whitespace-onlyThe fix matches the issue description: "the menu should only open when / is the first thing inputted" (meaning editor is otherwise empty).
can't this be done way simpler?
like, we only want to trigger if the first char in the first line is /, or what am i missing?
The user is right. Let me think about this more carefully.
The issue says: "the menu should only open when / is the first thing inputted"
So really, we just want to check if:
/Or even simpler: the entire editor content is just a slash command being typed, meaning:
/ (after trimming leading whitespace)Actually, the simplest interpretation is:
/ + command nameWait, let me re-read the original issue:
"adjust slash command autocomplete in the TUI editor so / only opens the command menu when the input is otherwise empty (no non-whitespace anywhere else in the message)"
So the user wants: slash menu only when the editor is "empty" except for the slash command itself.
The simplest check would be:
/But actually, if we're strict about "first thing inputted", it's even simpler:
lines[0].trimStart().startsWith("/")Or the SIMPLEST: just check if we're on line 0 and the trimmed content starts with /. If someone has typed on other lines, they'd have to be on those lines to type there, and when they come back to line 0 with a /, we probably still want to allow it if that's all that's on line 0...
Hmm, but the issue shows a screenshot where the user typed text, then went to a new line and typed /. The menu appeared but didn't work correctly.
The slash command needs to be the entire messageāyou can't mix it with other text. So I should check if all content across all lines, when combined and trimmed, is just the slash command pattern. This means joining all lines, trimming, and verifying it starts with / and has no other content, or checking that the first line starts with / and no other lines contain non-whitespace characters.
The user's approach is cleaner: if the first character of the trimmed first line is /, that's a slash command context. Once there's content on other lines, it's no longer a slash command. Looking at the current implementation, isSlashMenuAllowed blocks if there's non-whitespace after the cursor on the current line or if any other line has content. A simpler approach would be checking this.state.cursorLine === 0 && this.state.lines[0].trimStart().startsWith("/"). But I need to consider where these checks are called fromāisAtStartOfMessage() and isInSlashCommandContext() are used in different contexts, like when typing / to determine if the menu should appear. and no other content exists. If there's only one line, allow it. For multiple lines, check that all lines after the first are empty, and the cursor is on line 0. But this requires iteration through all lines.
The most straightforward approach is just checking if the cursor is on line 0āslash commands can only exist at the start of the editor, so if you're not on the first line, you can't be in a slash command context.
However, there's a complication: someone could type /model, press Enter to create a new line, then navigate back up to edit the command. In that case, we'd want autocomplete to still work. Similarly, if the first line contains /model but there's other text below it, autocomplete shouldn't trigger based on the issue requirements. So the logic needs to verify both that the cursor is on line 0 and that no other lines contain non-whitespace content. The user's suggestion about checking if the first character is / captures part of this, but we also need to ensure there's nothing else in the message. The simplest approach would be to check that every line is either the current cursor line or completely empty. This avoids the dual-check logic in the current implementation.
However, there's a trade-off: this still requires iterating through all lines. We could avoid iteration entirely by just checking this.state.cursorLine === 0, but that wouldn't catch the case where a user types on line 0, moves to another line, then comes back. The pragmatic question is whether the added complexity of tracking non-whitespace state elsewhere is worth avoiding the iteration.
Looking more carefully at what the check should actually do: if there are multiple lines with content, or if the cursor isn't on line 0, the slash menu shouldn't appear. There's also the question of whether content after the cursor position on the current line should block the menu. The current implementation seems to check what comes after the cursor specifically, not just whether the line has any content at all.
The logic appears to be: nothing can exist after the cursor position on the current line, and every other line must be empty. This prevents the menu from showing when you're editing in the middle of text or when there's content elsewhere in the input.
But for a slash command specifically, that "after cursor" restriction might be unnecessary. A simpler approach would just require being on line 0 with all other lines empty, which still prevents the menu from appearing when there's multi-line content. The original check seems overly restrictive for this use case.
You're right, it could be simpler. Looking at the issue again:
"the menu should only open when
/is the first thing inputted"
The simplest interpretation:
private isSlashMenuAllowed(): boolean { // Only allow slash menu on line 0 with no content on other lines if (this.state.cursorLine !== 0) return false; for (let i = 1; i < this.state.lines.length; i++) { if (this.state.lines[i].trim() !== "") return false; } return true;}Or even simpler if we're strict about "first thing":
private isSlashMenuAllowed(): boolean { return this.state.lines.length === 1 || this.state.lines.slice(1).every(l => l.trim() === "");}The current PR has an extra check for "non-whitespace after cursor on current line" which handles edge cases like cursor in the middle of hello /world, but that seems unnecessary for slash commands since they must start at the beginning anyway.
The question is whether cursorLine === 0 check is even needed. If you type / on line 3, the existing isAtStartOfMessage() check (beforeCursor.trim() === "" || beforeCursor.trim() === "/") would pass, but then the "other lines have content" check would fail.
So yes, the PR could be simplified. Want me to suggest a cleaner version to the contributor?
implement the most concise, correct fix locally then i test
The user wants me to implement a simpler fix locally. Let me think about the most concise correct solution.
The requirement is: slash menu should only trigger when the editor is "empty" except for the slash command being typed.
The simplest check:
That's it. We don't need to check cursor position on line 0 specifically, because:
/, and other lines are empty ā allow/, and line 0 has content ā block (other lines check fails)/, and all other lines are empty ā this is weird but harmless to allowActually wait, let me think about this more carefully. The original isAtStartOfMessage() checks:
return beforeCursor.trim() === "" || beforeCursor.trim() === "/";This ensures the slash is at the start of the line (after whitespace). So if you type hello /, this would return false and not trigger the menu.
The issue is when you have:
hello/Line 0: "hello" Line 1: "/" (cursor here)
With the original code, isAtStartOfMessage() would return true (beforeCursor is "/" which trims to "/"), and the menu would trigger incorrectly.
So the fix needs to gate this: only allow if other lines are empty.
The simplest implementation:
private isSlashMenuAllowed(): boolean { // Slash menu only when editor has no other content for (let i = 0; i < this.state.lines.length; i++) { if (i === this.state.cursorLine) continue; if (this.state.lines[i].trim() !== "") return false; } return true;}This is essentially what the PR has, minus the "afterCursor" check.
Now I need to check whether the afterCursor validation is actually necessary. If I'm on line 0 with hello /world and the cursor sits after the space, beforeCursor would be "hello " which fails the isAtStartOfMessage() check anyway, so the menu wouldn't trigger. The afterCursor check appears redundant since the existing logic already prevents triggering when there's content before the slash on the same line.
Looking at the different code paths: when typing /, isAtStartOfMessage() returns false if there's any non-whitespace before it. When typing alphanumerics in a slash context, isInSlashCommandContext() checks if the text before the cursor starts with / after trimming. For backspace and delete operations, the same context check applies. The afterCursor validation seems designed to handle edge cases like inserting a / in the middle of existing textāfor instance, if the cursor is positioned as hel|lo and you type /, you'd get hel/|lo.
In that scenario, isAtStartOfMessage() would evaluate "hel/" trimmed, which isn't empty or just /, so it returns false and the menu doesn't appear. This suggests the afterCursor check is actually redundant. A cleaner approach would be to consolidate the logic: create a method that checks if all other lines are empty, then use that within isAtStartOfMessage() and a new isInSlashCommandContext() method. The PR appears to modify isAtStartOfMessage() and introduce additional context checks. The PR refactors this by adding the isSlashMenuAllowed() check directly into isAtStartOfMessage(), then introduces a new isInSlashCommandContext() method that combines the permission check with a trimStart operation on the text before the cursor. This consolidates the slash command validation logic into dedicated helper methods rather than scattering these checks across multiple call sites. "/";
}
private isInSlashCommandContext(textBeforeCursor: string): boolean { return this.isSlashMenuAllowed() && textBeforeCursor.trimStart().startsWith("/"); }
I should update the four call sites that check `textBeforeCursor.trimStart().startsWith("/")` to use this new helper method instead, replacing them with `this.isInSlashCommandContext(textBeforeCursor)`. Then I'll merge these changes into the main branch.Now let me look at the places that need to be changed - the 4 call sites that check textBeforeCursor.trimStart().startsWith("/"):1. Around line 984-987 in insertCharacter()2. Around line 1169-1172 in handleBackspace()
3. Around line 1388-1391 in handleForwardDelete()
4. Around line 1848-1851 in handleTabCompletion() - I can see this one in the code I just readLet me find the exact locations:
>grep -n "trimStart().startsWith" packages/tui/src/components/editor.ts987: if (textBeforeCursor.trimStart().startsWith("/")) {
1172: if (textBeforeCursor.trimStart().startsWith("/")) {
1391: if (textBeforeCursor.trimStart().startsWith("/")) {
1851: if (beforeCursor.trimStart().startsWith("/") && !beforeCursor.trimStart().includes(" ")) {Now let me read the context around each of these lines to make precise edits.
Now I have all the context. Let me implement the simplest fix:
isSlashMenuAllowed() helper - checks that all other lines are emptyisAtStartOfMessage() to call the helper firstisInSlashCommandContext() helper that combines the checkLet me make the edits:
Now I'll implement the simplified fix:
Now I need to update the 4 call sites. The line numbers may have shifted slightly due to the edit. Let me check the new positions and update them.
>grep -n "trimStart().startsWith" packages/tui/src/components/editor.ts987: if (textBeforeCursor.trimStart().startsWith("/")) {
1172: if (textBeforeCursor.trimStart().startsWith("/")) {
1391: if (textBeforeCursor.trimStart().startsWith("/")) {
1823: return this.isSlashMenuAllowed() && textBeforeCursor.trimStart().startsWith("/");
1863: if (beforeCursor.trimStart().startsWith("/") && !beforeCursor.trimStart().includes(" ")) {Line 1823 is the new isInSlashCommandContext method I just added. Now I need to update lines 987, 1172, 1391, and 1863 to use this.isInSlashCommandContext() instead.
Let me update the next two occurrences.
Let me read that section to get the exact text.
I see the issue - this is a different location (inside handleBackspace). Let me fix this one specifically.
Now let me find and update the third occurrence (in handleForwardDelete).
Good, let me update this occurrence too.
Now I need to update the last occurrence in handleTabCompletion. Let me read that section.
Now I can update this last occurrence. I need to update it to use isInSlashCommandContext, but I also need to keep the !beforeCursor.trimStart().includes(" ") check. Let me update it appropriately.
Now let me verify there are no more occurrences to update and run the check command.
>grep -n "trimStart().startsWith" packages/tui/src/components/editor.ts1823: return this.isSlashMenuAllowed() && textBeforeCursor.trimStart().startsWith("/");Only one occurrence left, and that's in the isInSlashCommandContext method itself. Now let me run the check to make sure the code compiles correctly.
>cd /Users/badlogic/workspaces/pi-mono && npm run check 2>&1> pi-monorepo@0.0.3 check
> biome check --write . && tsgo --noEmit && npm run check -w @mariozechner/pi-web-ui
Checked 417 files in 264ms. No fixes applied.
> @mariozechner/pi-web-ui@0.49.3 check
> biome check --write . && tsc --noEmit && cd example && biome check --write . && tsc --noEmit
Checked 73 files in 37ms. No fixes applied.
Checked 3 files in 8ms. No fixes applied.All checks pass. Let me show the user what the final implementation looks like.
Done. Here's the simplified implementation:
// Slash menu only allowed when all other lines are empty (no mixed content)private isSlashMenuAllowed(): boolean { for (let i = 0; i < this.state.lines.length; i++) { if (i === this.state.cursorLine) continue; if (this.state.lines[i].trim() !== "") return false; } return true;}
// Helper method to check if cursor is at start of message (for slash command detection)private isAtStartOfMessage(): boolean { if (!this.isSlashMenuAllowed()) return false; const currentLine = this.state.lines[this.state.cursorLine] || ""; const beforeCursor = currentLine.slice(0, this.state.cursorCol); return beforeCursor.trim() === "" || beforeCursor.trim() === "/";}
private isInSlashCommandContext(textBeforeCursor: string): boolean { return this.isSlashMenuAllowed() && textBeforeCursor.trimStart().startsWith("/");}Differences from PR:
isAtStartOfMessage already handles that case)npm run check passes. Ready for you to test.
oh ok, so this doesn't prevent \n\n/ from triggering, but it does if i do
asf /, right?
wtf?
gah
Explaining slash menu logic
The slash menu is allowed only when all other lines are empty, meaning users can access it if they have blank lines present. If any line contains text, then using the slash menu is blocked.
With the current logic:
\n\n/ does trigger the menu because all other lines are empty.asf\n/ does not trigger because line 0 has non whitespace.So yes, blank lines still allow the slash menu. If you want to block \n\n/ and only allow / on the first line, we can add this.state.cursorLine === 0 to isSlashMenuAllowed().
no it's fine. commit and push, close pr with a thank you and that we implementd this manually in a more concise way
Preparing commit and branch workflow
I'm checking the current branch and staging the modified file, then creating a feature branch to commit changes before merging back to main and pushing, following the PR workflow and preparing to close the PR with a concise thank-you note.
Adding changelog entry for fix
Planning to add a "Fixed" section under Unreleased in the changelog with an entry referencing issue #904 to document the manual slash-menu fix before completing the branch commit and PR closure.
Confirming no rerun needed
The code checks ran after the actual code changes, so since only the changelog changed now, rerunning checks isn't necessary. Running git status next.
>cd /Users/badlogic/workspaces/pi-mono && git status --shortM packages/ai/src/models.generated.ts
M packages/tui/CHANGELOG.md
M packages/tui/src/components/editor.ts
?? .pi/git/
?? .pi/npm/Ensuring clean branch for commit
I'm creating a new branch to isolate our changes, avoiding unrelated modifications and excluding .pi/ files; this keeps the commit focused only on our new files.
>cd /Users/badlogic/workspaces/pi-mono && git checkout -b fix/slash-menu-trigger-cleanSwitched to a new branch 'fix/slash-menu-trigger-clean'Adding specific files
>cd /Users/badlogic/workspaces/pi-mono && git add packages/tui/src/components/editor.ts packages/tui/CHANGELOG.md(no output)>cd /Users/badlogic/workspaces/pi-mono && git commit -m "fix(tui): gate slash menu to empty input"Running formatting, linting, and type checking...
> pi-monorepo@0.0.3 check
> biome check --write . && tsgo --noEmit && npm run check -w @mariozechner/pi-web-ui
Checked 417 files in 295ms. No fixes applied.
> @mariozechner/pi-web-ui@0.49.3 check
> biome check --write . && tsc --noEmit && cd example && biome check --write . && tsc --noEmit
Checked 73 files in 44ms. No fixes applied.
Checked 3 files in 8ms. No fixes applied.
ā
All pre-commit checks passed!
[fix/slash-menu-trigger-clean f42deae1] fix(tui): gate slash menu to empty input
2 files changed, 22 insertions(+), 6 deletions(-)Preparing to merge while isolating main
I'm switching to the main branch, which has other agent changes, to avoid interfering with them before merging.
>cd /Users/badlogic/workspaces/pi-mono && git checkout mainSwitched to branch 'main'
M packages/ai/src/models.generated.ts
Your branch is up to date with 'origin/main'.Merging feature branch into main
>cd /Users/badlogic/workspaces/pi-mono && git merge --no-ff fix/slash-menu-trigger-cleanMerge made by the 'ort' strategy.
packages/tui/CHANGELOG.md | 4 ++++
packages/tui/src/components/editor.ts | 24 ++++++++++++++++++------
2 files changed, 22 insertions(+), 6 deletions(-)Preparing to push main
I'm confirming that only committed changes will be pushed, leaving uncommitted modified files unstaged and excluded from the push.
>cd /Users/badlogic/workspaces/pi-mono && git push origin mainTo https://github.com/badlogic/pi-mono.git
0f0c54b8..e13d97f7 main -> mainClosing PR with concise comment
I'm closing PR 907 with a comment thanking the contributor and noting the change was implemented manually in a more concise way on main, without emojis or mentioning the changelog.
>gh pr close 907 --repo badlogic/pi-mono --comment "Thanks @tmustier. We implemented this manually in a more concise way on main, so closing this PR."ā Closed pull request badlogic/pi-mono#907 (fix(tui): gate slash menu to empty input)Summarizing changes and tests
I'm preparing a concise summary mentioning the modified files and the tests run. I'll note the added changelog entry and consider whether to warn about uncommitted changes from others, leaning toward skipping that.
Summary:
packages/tui/src/components/editor.ts and reused it in autocomplete paths.packages/tui/CHANGELOG.md.Tests:
npm run checkprivate isSlashMenuAllowed(): boolean { const currentLine = this.state.lines[this.state.cursorLine] ?? ""; const afterCursor = currentLine.slice(this.state.cursorCol);
// Check 1: non-whitespace AFTER cursor on current line if (afterCursor.trim() !== "") { return false; }
// Check 2: non-whitespace on ANY OTHER line for (let i = 0; i < this.state.lines.length; i++) { if (i === this.state.cursorLine) continue; if ((this.state.lines[i] ?? "").trim() !== "") { return false; } } return true;}for (let i = 0; i < this.state.lines.length; i++) { if (i === this.state.cursorLine) continue; if ((this.state.lines[i] ?? "").trim() !== "") { return false; // "line" on line 0 triggers this }}private isSlashMenuAllowed(): boolean { // Only allow slash menu on line 0 with no content on other lines if (this.state.cursorLine !== 0) return false; for (let i = 1; i < this.state.lines.length; i++) { if (this.state.lines[i].trim() !== "") return false; } return true;}private isSlashMenuAllowed(): boolean { return this.state.lines.length === 1 || this.state.lines.slice(1).every(l => l.trim() === "");}return beforeCursor.trim() === "" || beforeCursor.trim() === "/";hello/private isSlashMenuAllowed(): boolean { // Slash menu only when editor has no other content for (let i = 0; i < this.state.lines.length; i++) { if (i === this.state.cursorLine) continue; if (this.state.lines[i].trim() !== "") return false; } return true;}
I should update the four call sites that check `textBeforeCursor.trimStart().startsWith("/")` to use this new helper method instead, replacing them with `this.isInSlashCommandContext(textBeforeCursor)`. Then I'll merge these changes into the main branch.// Slash menu only allowed when all other lines are empty (no mixed content)private isSlashMenuAllowed(): boolean { for (let i = 0; i < this.state.lines.length; i++) { if (i === this.state.cursorLine) continue; if (this.state.lines[i].trim() !== "") return false; } return true;}
// Helper method to check if cursor is at start of message (for slash command detection)private isAtStartOfMessage(): boolean { if (!this.isSlashMenuAllowed()) return false; const currentLine = this.state.lines[this.state.cursorLine] || ""; const beforeCursor = currentLine.slice(0, this.state.cursorCol); return beforeCursor.trim() === "" || beforeCursor.trim() === "/";}
private isInSlashCommandContext(textBeforeCursor: string): boolean { return this.isSlashMenuAllowed() && textBeforeCursor.trimStart().startsWith("/");}