fix(conversation): insert the selection popup text at the composer caret (#425)
appendToDraft always spliced at the end of the draft. The conversation service exposes no caret API, so probe the live composer textarea selection from the DOM (value-sync guarded) and splice at that position with whitespace-aware joins; an unknown or stale caret falls back to appending. The explorer @-reference shares this path, so it inserts at the caret too.
This commit is contained in:
懿驹-伯乐
@@ -1,16 +1,88 @@
|
||||
/**
|
||||
* Append text to the current session's composer draft through the
|
||||
* Insert text into the current session's composer draft through the
|
||||
* conversation service — the shared path behind the explorer's @-reference
|
||||
* button and the viewer selection popup. The service is resolved lazily
|
||||
* through `ctx.get` (the inject-free read the app's own plugins use); a
|
||||
* missing service or scope degrades to a logged no-op, never a crash.
|
||||
*
|
||||
* Insert position (fixes upstream issue #425): the draft store only exposes
|
||||
* the whole string (`getSnapshot().draft` + `setDraft(text)`) — there is no
|
||||
* caret API on the conversation service. The composer's `<textarea>` keeps
|
||||
* its last selection even while unfocused, so the live caret is probed from
|
||||
* the DOM (guarded by a value-sync check), and the text is spliced at that
|
||||
* position, replacing any live selection — with whitespace-aware joins, an
|
||||
* insert into the middle of a sentence keeps single-space separation like
|
||||
* the append path. An unknown/stale caret falls back to appending at the
|
||||
* end (the pre-fix behavior).
|
||||
*/
|
||||
import type { Context, SidebarConversation } from '../context-types.ts'
|
||||
|
||||
/** A resolved composer caret/selection in draft coordinates. */
|
||||
export interface DraftCaret {
|
||||
start: number
|
||||
end: number
|
||||
}
|
||||
|
||||
/**
|
||||
* Append `text` to the session's composer draft (space-separated, like the
|
||||
* @-mentions). Returns false — and logs — when the conversation service or
|
||||
* the session scope is unavailable.
|
||||
* Splice `text` into `draft` at `caret` (replacing any live selection) with
|
||||
* whitespace-aware joins. `caret === null` (position unknown) appends at the
|
||||
* end, exactly like the original behavior. Pure string math — unit-tested
|
||||
* directly.
|
||||
*/
|
||||
export function insertAtCaret(draft: string, text: string, caret: DraftCaret | null): string {
|
||||
if (caret === null || draft === '') {
|
||||
return draft.trim() === '' ? text : `${draft} ${text}`
|
||||
}
|
||||
const prefix = draft.slice(0, caret.start)
|
||||
const suffix = draft.slice(caret.end)
|
||||
if (prefix === '' && suffix === '') return text
|
||||
// One separating space, but never doubled against adjacent whitespace
|
||||
// (or the string edges) — mirrors how typing in the middle of a sentence
|
||||
// behaves.
|
||||
const left = prefix === '' || /\s$/.test(prefix) ? '' : ' '
|
||||
const right = suffix === '' || /^\s/.test(suffix) ? '' : ' '
|
||||
return `${prefix}${left}${text}${right}${suffix}`
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the composer's live caret from its DOM `<textarea>`. The draft
|
||||
* store has no caret API, so the sidebar reads the composed input's selection
|
||||
* directly; the value-sync check (`el.value === draft`) discards stale or
|
||||
* wrong-composer reads — a caret must never be applied against a draft it
|
||||
* was not measured on.
|
||||
*
|
||||
* Returns null when the composer is missing, disabled/read-only, out of
|
||||
* sync with the store draft, or has no measurable selection (jsdom/odd
|
||||
* hosts report null selectionStart/End).
|
||||
*/
|
||||
export function probeComposerCaret(draft: string): DraftCaret | null {
|
||||
if (typeof document === 'undefined') return null
|
||||
// The active composer lives in the conversation column; prefer its
|
||||
// `data-phase`-tagged textarea (the composer's marker), falling back to
|
||||
// any textarea in the column, then to a bare data-phase textarea (older
|
||||
// host layouts without the column attribute).
|
||||
const column = document.querySelector('#root [data-slot="conversation"]')
|
||||
const find = (scope: ParentNode): HTMLTextAreaElement | null =>
|
||||
scope.querySelector('textarea[data-phase]') ?? scope.querySelector('textarea')
|
||||
const el = column !== null
|
||||
? find(column)
|
||||
: document.querySelector<HTMLTextAreaElement>('textarea[data-phase]')
|
||||
if (el === null || el.disabled || el.readOnly) return null
|
||||
if (el.value !== draft) return null
|
||||
let start = el.selectionStart
|
||||
let end = el.selectionEnd
|
||||
if (typeof start !== 'number' || typeof end !== 'number') return null
|
||||
if (!Number.isFinite(start) || !Number.isFinite(end)) return null
|
||||
start = Math.max(0, Math.min(start, draft.length))
|
||||
end = Math.max(start, Math.min(end, draft.length))
|
||||
return { start, end }
|
||||
}
|
||||
|
||||
/**
|
||||
* Insert `text` into the session's composer draft at the composer's live
|
||||
* caret (see {@link probeComposerCaret}), falling back to appending at the
|
||||
* end when the caret cannot be resolved. Returns false — and logs — when the
|
||||
* conversation service or the session scope is unavailable.
|
||||
*/
|
||||
export function appendToDraft(ctx: Context, sessionId: string, text: string): boolean {
|
||||
try {
|
||||
@@ -20,10 +92,10 @@ export function appendToDraft(ctx: Context, sessionId: string, text: string): bo
|
||||
if (conversation === undefined) return false
|
||||
const input = conversation.input.for(actx)
|
||||
const draft = input.state.getSnapshot().draft
|
||||
input.setDraft(draft.trim() === '' ? text : `${draft} ${text}`)
|
||||
input.setDraft(insertAtCaret(draft, text, probeComposerCaret(draft)))
|
||||
return true
|
||||
} catch (error) {
|
||||
console.warn('[dsh-better-sidebar] draft insert failed:', error)
|
||||
return false
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,122 @@
|
||||
/**
|
||||
* conversation-draft spec — the caret-resolved composer insertion (upstream
|
||||
* issue #425 ②). `insertAtCaret` is pure string math (splice with
|
||||
* whitespace-aware joins); `probeComposerCaret` reads the live composer
|
||||
* `<textarea>` selection out of the DOM, guarded by a value-sync check so a
|
||||
* stale or wrong-composer caret is never applied.
|
||||
*/
|
||||
// @vitest-environment jsdom
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { insertAtCaret, probeComposerCaret } from '../src/client/conversation-draft.ts'
|
||||
|
||||
/**
|
||||
* Mount a fake composer textarea inside the DSH conversation column
|
||||
* (`#root [data-slot="conversation"] textarea[data-phase]`).
|
||||
*/
|
||||
function mountComposer(draft: string, selectionStart: number, selectionEnd: number): HTMLTextAreaElement {
|
||||
const root = document.createElement('div')
|
||||
root.id = 'root'
|
||||
const column = document.createElement('div')
|
||||
column.setAttribute('data-slot', 'conversation')
|
||||
const textarea = document.createElement('textarea')
|
||||
textarea.setAttribute('data-phase', 'plain')
|
||||
textarea.value = draft
|
||||
textarea.setSelectionRange(selectionStart, selectionEnd)
|
||||
column.append(textarea)
|
||||
root.append(column)
|
||||
document.body.append(root)
|
||||
return textarea
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
document.body.innerHTML = ''
|
||||
})
|
||||
|
||||
describe('probeComposerCaret', () => {
|
||||
it('returns the textarea selection when the DOM is in sync with the draft', () => {
|
||||
mountComposer('hello world', 5, 5)
|
||||
expect(probeComposerCaret('hello world')).toEqual({ start: 5, end: 5 })
|
||||
})
|
||||
|
||||
it('reports a live selection range, not just a collapsed caret', () => {
|
||||
mountComposer('hello world', 6, 11)
|
||||
expect(probeComposerCaret('hello world')).toEqual({ start: 6, end: 11 })
|
||||
})
|
||||
|
||||
it('returns null when no composer is mounted', () => {
|
||||
expect(document.querySelector('textarea')).toBeNull()
|
||||
expect(probeComposerCaret('anything')).toBeNull()
|
||||
})
|
||||
|
||||
it('returns null when the composer DOM value is out of sync with the store draft', () => {
|
||||
mountComposer('hello', 0, 0)
|
||||
expect(probeComposerCaret('hello changed on the server')).toBeNull()
|
||||
})
|
||||
|
||||
it('returns null for a disabled composer', () => {
|
||||
const input = mountComposer('hello', 2, 2)
|
||||
input.disabled = true
|
||||
expect(probeComposerCaret('hello')).toBeNull()
|
||||
})
|
||||
|
||||
it('returns null for a read-only composer', () => {
|
||||
const input = mountComposer('hello', 2, 2)
|
||||
input.readOnly = true
|
||||
expect(probeComposerCaret('hello')).toBeNull()
|
||||
})
|
||||
|
||||
it('clamps out-of-range selections into the draft bounds', () => {
|
||||
const input = mountComposer('hi', 0, 0)
|
||||
// Direct property writes (jsdom does not enforce bounds like browsers).
|
||||
input.selectionStart = 5
|
||||
input.selectionEnd = 9
|
||||
expect(probeComposerCaret('hi')).toEqual({ start: 2, end: 2 })
|
||||
})
|
||||
})
|
||||
|
||||
describe('insertAtCaret', () => {
|
||||
it('appends at the end when the caret is unknown (pre-fix behavior)', () => {
|
||||
expect(insertAtCaret('', 'X', null)).toBe('X')
|
||||
expect(insertAtCaret('hello', 'X', null)).toBe('hello X')
|
||||
expect(insertAtCaret(' ', 'X', null)).toBe('X')
|
||||
})
|
||||
|
||||
it('inserts at the caret in the middle of a sentence with one space each side', () => {
|
||||
expect(insertAtCaret('hello world', 'CODE', { start: 5, end: 5 })).toBe('hello CODE world')
|
||||
})
|
||||
|
||||
it('only adds the separating spaces the neighbors actually need', () => {
|
||||
expect(insertAtCaret('one two', 'CODE', { start: 3, end: 3 })).toBe('one CODE two')
|
||||
// Doubled gap, caret right after the word: the leading space is added,
|
||||
// the two after the caret stay untouched (like typing in the gap).
|
||||
expect(insertAtCaret('one two', 'CODE', { start: 3, end: 3 })).toBe('one CODE two')
|
||||
// Doubled gap, caret on the second space: neither side needs a new
|
||||
// space, both originals flank the insertion.
|
||||
expect(insertAtCaret('one two', 'CODE', { start: 4, end: 4 })).toBe('one CODE two')
|
||||
expect(insertAtCaret('one two ', 'CODE', { start: 8, end: 8 })).toBe('one two CODE')
|
||||
})
|
||||
|
||||
it('inserts at the start without a leading space', () => {
|
||||
expect(insertAtCaret('hello', 'CODE', { start: 0, end: 0 })).toBe('CODE hello')
|
||||
})
|
||||
|
||||
it('inserts at the end with a single trailing space', () => {
|
||||
expect(insertAtCaret('hello', 'CODE', { start: 5, end: 5 })).toBe('hello CODE')
|
||||
})
|
||||
|
||||
it('replaces the live selection', () => {
|
||||
expect(insertAtCaret('a little tale', 'CODE', { start: 2, end: 8 })).toBe('a CODE tale')
|
||||
})
|
||||
|
||||
it('replaces a selection surrounded by words with single-space joins', () => {
|
||||
expect(insertAtCaret('abc def ghi', 'CODE', { start: 4, end: 7 })).toBe('abc CODE ghi')
|
||||
})
|
||||
|
||||
it('replaces a selection spanning the whole draft', () => {
|
||||
expect(insertAtCaret('old', 'CODE', { start: 0, end: 3 })).toBe('CODE')
|
||||
})
|
||||
|
||||
it('handles an empty draft with a resolved caret', () => {
|
||||
expect(insertAtCaret('', 'CODE', { start: 0, end: 0 })).toBe('CODE')
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user