fix(web/shell): #107 review — Tab no longer completes to another file's name, misreads where a command starts, or leaves a stray quote

The five findings of the review on completion.js, each reproduced against the real modules first. Every new test
fails on e93bc09 and passes here.

1. A name with a character the editor does not type (U+200B and the rest of cleanText's invisible list) was offered:
   `rm a` + Tab gave `rm ab.txt ` for `a<ZWSP>b.txt`, another file's name, with only a dim note under the line. The
   completer now drops a candidate whose written-back text cleanText would change (completion.js createCompleter).
   Names whose invisible characters the editor keeps (an emoji's selector, a ZWJ family, a Persian ZWNJ) are offered
   as before.

2. A line end that is not quoted or escaped was read as a blank, so in a pasted `echo hi\ngre` the word `gre` asked
   for paths. A line end now ends a command, as `;` does (wordAt).

3. A leading assignment took the command position: `LC_ALL=C gre` asked for paths. A word that is `NAME=`, `NAME+=`
   or `NAME[i]=`, judged on the raw token so the name has to be unquoted as bash requires (`"LC_ALL"=C` is a
   command word), keeps the command position (wordAt, ASSIGNMENT).

4. A word written against `<` or `>` was dropped without ending the command position: `cat>out sr` asked for commands.
   The word now ends there like any other unless it is all digits (the `2` of `2>err`). `>&`, `<&` and `>|` (an `&`
   or `|` right after the operator) and `&>`, `&>>` are read as redirections, not as a list or a pipe: `2>&1 gre`
   and `>|out gre` complete commands, `cat &>out sr` a file. Every row of the new table was checked with `bash -xc`
   (bash 3.2, the only bash on this machine: Docker was down, so not the golden suite's 5.1).

5. Tab just before a closing quote left the quote behind: `cat "my fi|"` became `cat "my file.txt" "`, an open quote,
   and the line was held for more. GNU readline does no better (`cat "my file.txt""`), so bash is not the model here.
   wordAt now also returns where the word ends after the cursor and what it holds there. The completer offers only
   names that end with that rest (any name when the rest is empty) and gives each such candidate an `end`. The
   editor's `_replace` replaces through that end, so the closing quote and the tail of a name typed earlier are
   consumed, never split off. A folder at the end of a word still keeps its quote open, with the cursor before the
   quote that is there. In the editor (line-editor.js `_complete`), too: a file gets no blank when one follows already;
   a common part that is a whole candidate (`'it'` of `it` and `it's`) takes that candidate's end (`cat 'i|'` gave
   `cat 'it''`); an `end` before the cursor or past the line completes nothing. A cursor at the start of a word still
   inserts a word of its own (`cat |foo` gives `cat x.txt foo`, as before).

NOTES.md: the completion section says what counts as a command position and what Tab inside a word does, and no
longer that a name it cannot type "is typed without" the character.

Author: ndemianc

web/shell/src/terminal/NOTES.md

@@ -109,11 +109,23 @@ names: the working folder comes from the engine, and a command can make a direct
 
 ### Completion
 
-Tab asks `session.complete('command' | 'path', prefix)`. One candidate is inserted (a folder gets no trailing space),
-several insert their common prefix, and a second Tab with nothing to add lists them in columns below and redraws the
-prompt. Names are written back escaped or quoted for the quote the word is in. A name that cannot be typed (a control
-character, a bidi override, any invisible character) is not offered or is typed without it. An answer that arrives after
-the line changed is ignored.
+Tab asks `session.complete('command' | 'path', prefix)` about what is before the cursor. It asks for commands for the
+first word of a command: at the start of the line, after `;` `|` `&` `(` and a line end that is not quoted or escaped,
+after `if` `then` `do` `!` and the like, and after what can come before a command without being it: assignments
+(`LC_ALL=C gre`; the name unquoted, as bash reads one) and redirections with their targets (`2>err`, `>out`, `2>&1`,
+`&>out`, `>|out`). A word written against a redirection is a word (`cat>out sr` completes a file: `cat` is the command),
+unless it is digits (the `2` of `2>err`). Checked against bash with `bash -xc`. Everything else asks for paths.
+
+One candidate is inserted (a folder gets no trailing space; a file gets one unless a blank follows it already), several
+insert their common prefix, and a second Tab with nothing to add lists them in columns below and redraws the prompt.
+Names are written back escaped or quoted for the quote the word is in. **Tab inside a word** (anything after the cursor,
+a closing quote too) offers only the names that end with what the word has after the cursor, and such a name replaces
+the whole word: `cat "my fi|"` becomes `cat "my file.txt" `, not `cat "my file.txt" "` (bash's own readline leaves
+`cat "my file.txt""`), and nothing that was after the cursor is dropped or split off. A folder at the end of a word
+keeps its quote open, with the cursor before the closing quote that is there. A name that cannot be typed (a control
+character, a bidi override, any invisible character the editor would not type) is not offered: typed without it, it is
+another file's name (`rm a` must not become `rm ab.txt` for `a<ZWSP>b.txt`). A name whose invisible characters the editor
+keeps (an emoji's, a joiner inside a word) is offered. An answer that arrives after the line changed is ignored.
 
 A Tab pressed before the answer to the previous one has arrived is **queued** (one at most; a third is dropped): it asks
 when that answer has been applied, about the line as it is then, so `Tab Tab` lists even when the first answer is slow.
@@ -419,7 +431,7 @@ Measured in the release build by the research, and re-checked here against the r
 | `width` | the 25 measured widths; the table equals the addon (when an xterm checkout is found) |
 | `line-editor` | the keys, the cursor on a newline, the full row, wide/combining characters, what may enter the buffer and what is kept (and the note), tabs (also at the top of a window), resize (scenarios on the model terminal, and the same on xterm.js), a tall buffer, Tab and the queued second Tab; a seeded differential against an oracle at three widths and with resizes; and, if xterm is found, against the real xterm.js: the editor's output, the model terminal's re-wrapping, seeded sessions with resizes in both directions, and a strict harness (after every step the screen is the text written once, nothing of the region is above it, and nothing that was on the terminal before it is lost: shrink and grow, a command, a note, Ctrl+C, Ctrl+L, windows, pinned sequences; and width and height in one event, with earlier output taller than the panel: pinned corner scenarios, each of which guards one of the estimates, and 200 seeded sessions on each terminal, aimed at the cursor above the end of a region on a terminal that has scrolled, in which a copy of the prompt may be left but nothing before it may be lost; then the same scenarios again through the verifier's generator's checker, which asks after every step that what the editor believes of the blank rows and of the pushed rows is a floor and that no copy of a pasted line is left (the corner ones), and 683 of its sessions in about two seconds; and the account of the pushed rows itself, `_ghostsAt` and `_headRows`, as units) |
 | `completeness` | the table of traps and open constructs |
-| `completion` | the word under the cursor and the write-back |
+| `completion` | the word under the cursor (command or path: line ends, assignments, redirections against a word; the rest of the word after the cursor), the write-back, the names that are not offered |
 | `output-filter` | the 25 queries, the modes, the OSC list, links, the cap, a seeded chunking fuzz |
 | `history` | the rules and the secret patterns (assembled from fragments) |
 | `sink` | the OSC 633 escaping, the nonce, the queue and a run's output |

web/shell/src/terminal/completion.js

@@ -5,36 +5,60 @@
 // other word the entries of the directory the typed prefix points into, keeping what was typed before the name and
 // ending a folder with '/'. What this module adds is the shell's part: the word is read the way the shell reads it
 // (quotes and backslashes), and a candidate is written back quoted or escaped so the line still means what it says.
+// Tab inside a word (something after the cursor) offers only names that end with the rest of the word, and such a name
+// replaces the whole word, its closing quote too: what was after the cursor is never dropped, split off or left over.
 // @ts-check
 'use strict';
 
+const { cleanText } = require('./line-editor');
+
 /** Words after which the next word is a command again. */
 const COMMAND_WORDS = new Set(['if', 'then', 'else', 'elif', 'do', 'while', 'until', 'time', '!', '{']);
+/** A word that assigns a variable (`NAME=`, `NAME+=`, `NAME[i]=`, the name as typed: unquoted): before the command it is not one. */
+const ASSIGNMENT = /^[A-Za-z_][A-Za-z0-9_]*(?:\[[^\]]*\])?\+?=/;
+/** A word of digits written against `<` or `>` is the descriptor it redirects (`2>err`), not a word of the command. */
+const DESCRIPTOR = /^[0-9]+$/;
+/** What ends a word that is not quoted or escaped. */
+const WORD_END = /[ \t\n;|&()<>`]/;
 /** Characters that need a backslash in an unquoted word, wherever they are. */
 const ALWAYS_SPECIAL = /[\s"'\\$`&|;()<>*?[\]{}]/;
 /** Characters that need one only at the start of a word. */
 const START_SPECIAL = /[~#!^]/;
 const CONTROL = /[\x00-\x1f\x7f-\x9f\u202a-\u202e\u2066-\u2069]/;
 
 /**
- * The word the cursor is in (or the empty word it is about to start).
+ * The word the cursor is in (or the empty word it is about to start), and the rest of it after the cursor.
  * @param {string} line
  * @param {number} cursor  in code points
- * @returns {{ start: number, prefix: string, quote: '' | '"' | "'", kind: 'command' | 'path' }}
+ * @returns {{ start: number, prefix: string, quote: '' | '"' | "'", kind: 'command' | 'path', end: number, rest: string }}
+ *   `end` is where the word ends after the cursor, and `rest` what it holds there (unquoted, as `prefix` is)
  */
 function wordAt(line, cursor) {
-	const chars = Array.from(line).slice(0, cursor);
+	const all = Array.from(line);
+	const chars = all.slice(0, cursor);
 	const n = chars.length;
 	let start = 0;
 	let inWord = false;
 	let prefix = '';
 	let quote = /** @type {'' | '"' | "'"} */ ('');
+	let escaped = false;        // the last character before the cursor is a backslash: it escapes the one after it
 	let cmdPos = true;          // the word being read is the first of a command
 	let redirect = false;       // the word being read is the target of > or <
+	let opAt = -2;              // where the last `<` or `>` is: `>&`, `<&` and `>|` are one operator with it
 
 	const begin = (/** @type {number} */ i) => {
 		if (!inWord) { inWord = true; start = i; }
 	};
+	// The word that ends at `i`. The first word of a command takes the command position with it; the target of a
+	// redirection, a keyword (`if`, `!`) and an assignment before the command (`LC_ALL=C`) do not.
+	const endWord = (/** @type {number} */ i) => {
+		if (inWord) {
+			if (cmdPos && !redirect && !COMMAND_WORDS.has(prefix) && !ASSIGNMENT.test(chars.slice(start, i).join(''))) { cmdPos = false; }
+			redirect = false;
+		}
+		inWord = false;
+		prefix = '';
+	};
 
 	for (let i = 0; i < n; i++) {
 		const c = chars[i];
@@ -48,38 +72,74 @@ function wordAt(line, cursor) {
 		}
 		if (c === '\\') {
 			begin(i);
-			if (i + 1 < n) { prefix += chars[++i]; }
+			if (i + 1 < n) { prefix += chars[++i]; } else { escaped = true; }
 			continue;
 		}
 		if (c === "'" || c === '"') { begin(i); quote = c; continue; }
-		if (c === ' ' || c === '\t' || c === '\n') {
-			if (inWord) {
-				if (cmdPos && !redirect && !COMMAND_WORDS.has(prefix)) { cmdPos = false; }
-				redirect = false;
-			}
-			inWord = false;
-			prefix = '';
-			continue;
-		}
-		if (c === ';' || c === '|' || c === '&' || c === '(') {
+		if (c === ' ' || c === '\t') { endWord(i); continue; }
+		if ((c === '&' || c === '|') && opAt === i - 1) { continue; }        // `>&2`, `<&0`, `>|`: the rest of the operator
+		if (c === '&' && all[i + 1] === '>') { endWord(i); continue; }      // `&>`, `&>>`: a redirection, not a list
+		if (c === ';' || c === '|' || c === '&' || c === '(' || c === '\n') {
+			// the end of a command (a line end too, when it is not quoted or escaped): what follows is a command again
 			inWord = false;
 			prefix = '';
 			cmdPos = true;
 			redirect = false;
 			continue;
 		}
 		if (c === '<' || c === '>') {
-			inWord = false;
-			prefix = '';
+			if (inWord && !redirect && DESCRIPTOR.test(chars.slice(start, i).join(''))) {
+				inWord = false;
+				prefix = '';
+			} else {
+				endWord(i);                                                  // `cat>out`: `cat` is the command
+			}
 			redirect = true;
+			opAt = i;
 			continue;
 		}
 		begin(i);
 		prefix += c;
 	}
 	if (!inWord) { start = n; prefix = ''; }
 	const kind = cmdPos && !redirect && !prefix.includes('/') && !/^[.~]/.test(prefix) ? 'command' : 'path';
-	return { start, prefix, quote, kind };
+
+	// The rest of the word, read the same way, from the quote the cursor is in. A cursor that is not in a word (after a
+	// blank, at the start of the line) starts a new one: what follows it is another word, not the rest of this one.
+	let q = quote;
+	let rest = '';
+	let k = n;
+	if (escaped && k < all.length) { rest += all[k++]; }
+	for (; inWord && k < all.length; k++) {
+		const c = all[k];
+		if (q === "'") {
+			if (c === "'") { q = ''; } else { rest += c; }
+			continue;
+		}
+		if (q === '"') {
+			if (c === '\\' && k + 1 < all.length && '"\\$`'.includes(all[k + 1])) { rest += all[++k]; } else if (c === '"') { q = ''; } else { rest += c; }
+			continue;
+		}
+		if (c === '\\') {
+			if (k + 1 < all.length) { rest += all[++k]; }
+			continue;
+		}
+		if (c === "'" || c === '"') { q = c; continue; }
+		if (WORD_END.test(c)) { break; }
+		rest += c;
+	}
+	return { start, prefix, quote, kind, end: k, rest };
+}
+
+/**
+ * Does `name` hold the whole word: what was typed before the cursor at its start and what is after it at its end, apart
+ * (a folder's name with or without its '/')?
+ * @param {string} name @param {string} prefix @param {string} rest
+ */
+function fits(name, prefix, rest) {
+	if (rest === '') { return true; }
+	const base = name.endsWith('/') && !rest.endsWith('/') ? name.slice(0, -1) : name;
+	return base.length >= prefix.length + rest.length && base.endsWith(rest);
 }
 
 /**
@@ -104,7 +164,8 @@ function writeBack(name, quote, dir) {
  * The editor's completion hook, over a session.
  * @param {{ complete(kind: 'path' | 'command', prefix: string): Promise<string[]> }} session
  * @param {{ forTerminal(text: string, o?: { crlf?: boolean }): string }} sanitize
- * @returns {(line: string, cursor: number) => Promise<{ start: number, candidates: Array<{ text: string, display: string, dir: boolean }> } | null>}
+ * @returns {(line: string, cursor: number) => Promise<{ start: number, candidates: Array<{ text: string, display: string, dir: boolean, end?: number }> } | null>}
+ *   a candidate with an `end` replaces the word up to there, past the cursor; one without, up to the cursor
  */
 function createCompleter(session, sanitize) {
 	return async function complete(line, cursor) {
@@ -117,11 +178,21 @@ function createCompleter(session, sanitize) {
 		const candidates = [];
 		for (const name of names) {
 			if (typeof name !== 'string' || name === '' || CONTROL.test(name) || seen.has(name)) { continue; }
-			// a candidate must continue what was typed
-			if (!name.startsWith(w.prefix)) { continue; }
+			// a candidate must continue what was typed, and end with what the word has after the cursor
+			if (!name.startsWith(w.prefix) || !fits(name, w.prefix, w.rest)) { continue; }
 			seen.add(name);
 			const dir = name.endsWith('/');
-			candidates.push({ text: writeBack(name, w.quote, dir), display: sanitize.forTerminal(name), dir });
+			// A folder at the end of the word is left open for the next name; anything else is written whole and replaces
+			// the rest of the word (a closing quote after the cursor, the end of a name typed earlier).
+			const open = dir && w.rest === '';
+			const text = writeBack(name, w.quote, open);
+			// The editor types only what the screen shows: a name it would type without one of its characters (a zero-width
+			// space) is another file's name, so it is not offered.
+			if (cleanText(text).cps.join('') !== text) { continue; }
+			/** @type {{ text: string, display: string, dir: boolean, end?: number }} */
+			const candidate = { text, display: sanitize.forTerminal(name), dir };
+			if (!open && w.end > cursor) { candidate.end = w.end; }
+			candidates.push(candidate);
 		}
 		if (!candidates.length) { return null; }
 		// The word as typed begins with its own opening quote; the candidates repeat it, so replace from the word start.

web/shell/src/terminal/line-editor.js

@@ -118,7 +118,9 @@ function droppedNote(n) {
 }
 
 /**
- * @typedef {{ text: string, display?: string, dir?: boolean }} Candidate
+ * A candidate replaces the line from the answer's `start` up to the cursor, or up to its own `end` past it (the rest of
+ * the word: a closing quote, the end of a name), so that what was after the cursor is not left over.
+ * @typedef {{ text: string, display?: string, dir?: boolean, end?: number }} Candidate
  * @typedef {{
  *   write(s: string): void,
  *   prompt(): string,
@@ -1069,9 +1071,14 @@ class LineEditor {
 		if (!this.active || version !== this.version || !r || !r.candidates || r.candidates.length === 0) { return; }
 		if (!(r.start >= 0 && r.start <= this.cur)) { return; }
 		const cs = r.candidates;
+		const upTo = (/** @type {Candidate} */ c) => (c.end === undefined ? this.cur : c.end);
+		if (!cs.every((c) => upTo(c) >= this.cur && upTo(c) <= this.buf.length)) { return; }
 		const typed = this.buf.slice(r.start, this.cur).join('');
 		if (cs.length === 1) {
-			this._replace(r.start, cs[0].text + (cs[0].dir ? '' : ' '));
+			const end = upTo(cs[0]);
+			// a file is followed by a blank, unless the one that separates it from the next word is there already
+			const blank = cs[0].dir || /^[ \t\n]$/.test(this.buf[end] || '') ? '' : ' ';
+			this._replace(r.start, cs[0].text + blank, end);
 			return;
 		}
 		const common = cs.map((c) => c.text).reduce((a, b) => {
@@ -1081,18 +1088,20 @@ class LineEditor {
 		});
 		// still ambiguous after this Tab: the next one, if nothing was typed, lists the candidates
 		if (common.length > typed.length) {
-			this._replace(r.start, common);
+			// a common part that is a whole candidate (`'it'`, of it and it's) is that one: it replaces what it would
+			const whole = cs.find((c) => c.text === common);
+			this._replace(r.start, common, whole ? upTo(whole) : this.cur);
 			this.lastTab = true;
 			return;
 		}
 		if (!second) { this.lastTab = true; return; }
 		this._list(cs.map((c) => (c.display !== undefined ? c.display : c.text)));
 	}
 
-	/** @param {number} start @param {string} text */
-	_replace(start, text) {
-		const { cps, dropped } = cleanText(text, this.buf[start - 1], this.buf[this.cur]);
-		this._put(start, this.cur - start, cps);
+	/** @param {number} start @param {string} text @param {number} [end] where the text it replaces ends: the cursor, or past it */
+	_replace(start, text, end = this.cur) {
+		const { cps, dropped } = cleanText(text, this.buf[start - 1], this.buf[end]);
+		this._put(start, end - start, cps);
 		this.cur = start + cps.length;
 		this._changed(dropped);
 	}