fix(desktop): keep the browse scope through Tab, Backspace, and a mid-message pick

Three defects in the commit engine, all from treating the scope as loose text:

Tab-descend rebuilt the token as a bare `@apps/desktop/`, silently widening an
explicit @folder: browse back to files and forcing the committed chip to
re-guess its kind from a trailing slash. It now carries the scope down.

Backspace only handled a path, so at `@folder:` it fell through to character
deletion and nibbled back out through the directive syntax one key at a time.
It now drops the scope as one unit, mirroring Tab's one-key descent.

The rebuild fallback sliced tokenLength off the END of the draft, which assumed
the trigger was the last thing in the editor — a pick made mid-message chopped
the trailing prose off and stranded a partial `folder:` in front of the chip.
rebuildAroundCaret splits around the caret instead, and is shared with the
ascend path and the edit composer rather than hand-rolled three times.

Also drop the chip's auto-inserted trailing space when the caret already has
whitespace after it, so a mid-sentence pick doesn't leave a double space.
This commit is contained in:
Brooklyn Nicholson 2026-07-30 02:04:40 -05:00
parent fb5efe917a
commit 4fb4d78989

View file

@ -12,15 +12,59 @@ import {
slashCommandToken
} from '../composer-utils'
import {
appendComposerContents,
caretOffsetInEditor,
composerPlainText,
placeCaretEnd,
placeCaretAtOffset,
refChipElement,
renderComposerContents,
replaceBeforeCaret,
RICH_INPUT_SLOT,
slashChipElement
} from '../rich-editor'
import { detectTrigger, textBeforeCaret, type TriggerState } from '../text-utils'
/**
* Rebuild-from-text fallback for carets the range walk can't anchor (a
* non-collapsed selection, a caret not preceded by contiguous text). It
* re-renders the whole editor from serialized text, so it only runs when the
* in-place path reports failure — never as the default.
*
* The split is around the CARET, not the end of the draft. Slicing
* `length - tokenLength` off the end assumed the trigger token was the last
* thing in the editor: a completion picked mid-message chopped the trailing
* prose off and stranded a partial `folder:` in front of the chip, because the
* window it removed wasn't the token the user was typing.
*/
export function rebuildAroundCaret(editor: HTMLDivElement, tokenLength: number, insert: DocumentFragment | string) {
const current = composerPlainText(editor)
const caret = caretOffsetInEditor(editor)
const prefix = current.slice(0, Math.max(0, caret - tokenLength))
const suffix = current.slice(caret)
if (typeof insert === 'string') {
renderComposerContents(editor, `${prefix}${insert}${suffix}`)
placeCaretAtOffset(editor, prefix.length + insert.length)
return
}
// Measure before appending — moving a fragment empties it. Appending the
// element rather than re-serializing keeps mid-message slash pills alive:
// they have no text hydration, unlike `@` refs and the leading command.
const scratch = document.createElement('div')
scratch.dataset.slot = RICH_INPUT_SLOT
scratch.append(insert.cloneNode(true))
const inserted = composerPlainText(scratch)
renderComposerContents(editor, prefix)
editor.append(insert)
appendComposerContents(editor, suffix)
placeCaretAtOffset(editor, prefix.length + inserted.length)
}
interface CompletionSource {
adapter: Unstable_TriggerAdapter | null
loading: boolean
@ -238,16 +282,8 @@ export function useComposerTrigger({
// and a pick must be exactly one undo step.
recordUndoPoint?.()
// Rebuild-from-text fallback for carets the range walk can't anchor (a
// non-collapsed selection, a caret not preceded by contiguous text). It
// re-renders the whole editor from serialized text, so it only runs when
// the in-place path reports failure — never as the default.
const rebuildWith = (render: (prefix: string) => void) => {
const current = composerPlainText(editor)
render(current.slice(0, Math.max(0, current.length - trigger.tokenLength)))
placeCaretEnd(editor)
}
const rebuildAround = (insert: DocumentFragment | string) =>
rebuildAroundCaret(editor, trigger.tokenLength, insert)
// Action items (e.g. "Browse all sessions…") run a side effect instead of
// inserting a chip: strip the typed trigger token, then fire the action.
@ -256,7 +292,7 @@ export function useComposerTrigger({
if (runAction) {
if (!replaceBeforeCaret(editor, trigger.tokenLength, document.createDocumentFragment())) {
rebuildWith(prefix => renderComposerContents(editor, prefix))
rebuildAround('')
}
draftRef.current = composerPlainText(editor)
@ -290,12 +326,17 @@ export function useComposerTrigger({
if (descendInto) {
const path = descendInto.endsWith('/') ? descendInto : `${descendInto}/`
// Carry the browse scope down with the path. Dropping it turned an
// explicit `@folder:` browse into a bare `@apps/desktop/` token halfway
// through, so the next completion silently widened back to files and the
// committed chip had to re-guess the kind from a trailing slash.
const scope = trigger.scope ? `${trigger.scope}:` : ''
const fragment = document.createDocumentFragment()
fragment.append(document.createTextNode(`@${path}`))
fragment.append(document.createTextNode(`@${scope}${path}`))
if (!replaceBeforeCaret(editor, trigger.tokenLength, fragment)) {
rebuildWith(prefix => renderComposerContents(editor, `${prefix}@${path}`))
rebuildAround(`@${scope}${path}`)
}
return finish(true)
@ -325,59 +366,65 @@ export function useComposerTrigger({
? refChipElement(directive[1], directive[2])
: null
// The trailing space is a convenience for "keep typing after the chip", so
// it's wrong when the caret already has whitespace in front of it — a pick
// made mid-sentence would leave a double space in the prose.
const followedBySpace = /^\s/.test(composerPlainText(editor).slice(caretOffsetInEditor(editor)))
const fragment = document.createDocumentFragment()
chip ? fragment.append(chip, document.createTextNode(' ')) : fragment.append(document.createTextNode(text))
chip
? fragment.append(chip, ...(followedBySpace ? [] : [document.createTextNode(' ')]))
: fragment.append(document.createTextNode(followedBySpace ? text.trimEnd() : text))
if (!replaceBeforeCaret(editor, trigger.tokenLength, fragment)) {
rebuildWith(prefix => {
if (chip) {
// The failed in-place attempt never consumed the fragment, so the
// chip + trailing space land here instead. Appending the element
// keeps mid-message slash pills alive — they have no text
// hydration, unlike `@` refs and the leading command.
renderComposerContents(editor, prefix)
editor.append(fragment)
} else {
renderComposerContents(editor, `${prefix}${text}`)
}
})
// The failed in-place attempt never consumed the fragment, so the chip +
// trailing space are re-inserted around the caret here. Moving the
// element (rather than re-serializing) keeps mid-message slash pills
// alive — they have no text hydration, unlike `@` refs and the leading
// command.
rebuildAround(chip ? fragment : text)
}
finish(keepTriggerOpen)
}
/** Backspace inside an `@` path drops the last segment (`a/b/` → `a/`)
* instead of one character. Descending is one Tab per level, so climbing
* back out should cost one key too rather than a held delete. Returns
* instead of one character, and once the path is empty it drops the browse
* scope (`@folder:` → `@`) rather than nibbling `:`, `r`, `e`, `d`… back
* through the directive syntax the user never typed. Descending is one Tab
* per level, so climbing back out costs one key per level too. Returns
* false when the caret isn't in a path, so keydown falls through. */
const ascendTriggerPath = () => {
const editor = editorRef.current
if (!editor || trigger?.kind !== '@' || !trigger.query.includes('/')) {
if (!editor || trigger?.kind !== '@') {
return false
}
const scope = trigger.scope ? `${trigger.scope}:` : ''
if (!trigger.value.includes('/') && !scope) {
return false
}
// Trailing slash means we're listing a folder's children: drop that
// folder. Otherwise a partial segment is typed — drop just that.
const trimmed = trigger.query.replace(/\/$/, '')
// folder. Otherwise a partial segment is typed — drop just that. With the
// value already empty, the only thing left to drop is the scope itself.
const trimmed = trigger.value.replace(/\/$/, '')
const parent = trimmed.slice(0, trimmed.lastIndexOf('/') + 1)
const next = trigger.value ? `${scope}${parent}` : ''
recordUndoPoint?.()
const fragment = document.createDocumentFragment()
fragment.append(document.createTextNode(`@${parent}`))
fragment.append(document.createTextNode(`@${next}`))
// In place first: the destructive re-render fallback rebuilds the editor
// from text, which is exactly what used to demote a leading command pill
// to plaintext on every Backspace inside a path.
if (!replaceBeforeCaret(editor, trigger.tokenLength, fragment)) {
const current = composerPlainText(editor)
const prefix = current.slice(0, Math.max(0, current.length - trigger.tokenLength))
renderComposerContents(editor, `${prefix}@${parent}`)
placeCaretEnd(editor)
rebuildAroundCaret(editor, trigger.tokenLength, `@${next}`)
}
draftRef.current = composerPlainText(editor)