Fix review issues: gofmt, dead code, panics, typos, stale docs

rsteube · rsteube · commit 07a98366be54 · 2026-06-30T20:17:43.000+02:00
Code fixes: - Remove dead `wordchars` field from zshFormat - Remove dead `_ = err` and `_ = cursor` in completion.go - Guard WordbreakPrefix() against empty TokenSlice panic - Fix fish comment: `not` is not a pipeline delimiter (not in KeywordOperators) - Fix comment typos: "lexographical"→"lexicographic", "classifiees"→"classifies" - Fix wordbreak.go URL: stray `)` in Lists.html link - Fix stray blank lines in wordbreak.go and tokenslice.go - Apply gofmt -s across all files Doc fixes: - Update plan.md: TokenSlice methods stay exported (not unexported) - Update plan.md risk #7: remove "become internal implementation details" Assisted-by: Crush:glm-5.2
diff --git a/completion.go b/completion.go
@@ -84,8 +84,5 @@ func SplitForCompletionAt(s string, cursor int, format Format) *CompletionContex
 	}
 
 	ctx.Prefix = pipeline.WordbreakPrefix()
-
-	_ = cursor // cursor position is implicitly at end (tokens already reflect full input)
-	_ = err
 	return ctx
 }
diff --git a/completion_test.go b/completion_test.go
@@ -4,87 +4,87 @@ import "testing"
 
 func TestSplitForCompletion(t *testing.T) {
 	tests := []struct {
-		name     string
-		input    string
-		format   Format
-		wantWord string
-		wantPrefix string
-		wantState LexerState
+		name         string
+		input        string
+		format       Format
+		wantWord     string
+		wantPrefix   string
+		wantState    LexerState
 		wantRedirect bool
-		wantWords []string
+		wantWords    []string
 	}{
 		{
-			name: "simple word",
-			input: "echo hel",
-			format: BashFormat(),
-			wantWord: "hel",
+			name:       "simple word",
+			input:      "echo hel",
+			format:     BashFormat(),
+			wantWord:   "hel",
 			wantPrefix: "",
-			wantState: IN_WORD_STATE,
-			wantWords: []string{"echo", "hel"},
+			wantState:  IN_WORD_STATE,
+			wantWords:  []string{"echo", "hel"},
 		},
 		{
-			name: "inside double quotes",
-			input: `echo "hel`,
-			format: BashFormat(),
-			wantWord: "hel",
+			name:       "inside double quotes",
+			input:      `echo "hel`,
+			format:     BashFormat(),
+			wantWord:   "hel",
 			wantPrefix: "",
-			wantState: QUOTING_ESCAPING_STATE,
-			wantWords: []string{"echo", "hel"},
+			wantState:  QUOTING_ESCAPING_STATE,
+			wantWords:  []string{"echo", "hel"},
 		},
 		{
-			name: "inside single quotes",
-			input: "echo 'hel",
-			format: BashFormat(),
-			wantWord: "hel",
+			name:       "inside single quotes",
+			input:      "echo 'hel",
+			format:     BashFormat(),
+			wantWord:   "hel",
 			wantPrefix: "",
-			wantState: QUOTING_STATE,
-			wantWords: []string{"echo", "hel"},
+			wantState:  QUOTING_STATE,
+			wantWords:  []string{"echo", "hel"},
 		},
 		{
-			name: "pipeline",
-			input: "echo foo | grep bar",
-			format: BashFormat(),
-			wantWord: "bar",
+			name:       "pipeline",
+			input:      "echo foo | grep bar",
+			format:     BashFormat(),
+			wantWord:   "bar",
 			wantPrefix: "",
-			wantState: IN_WORD_STATE,
-			wantWords: []string{"grep", "bar"},
+			wantState:  IN_WORD_STATE,
+			wantWords:  []string{"grep", "bar"},
 		},
 		{
-			name: "redirect target",
-			input: "echo foo > bar",
-			format: BashFormat(),
-			wantWord: "bar",
-			wantPrefix: "",
-			wantState: IN_WORD_STATE,
+			name:         "redirect target",
+			input:        "echo foo > bar",
+			format:       BashFormat(),
+			wantWord:     "bar",
+			wantPrefix:   "",
+			wantState:    IN_WORD_STATE,
 			wantRedirect: true,
-			wantWords: []string{"echo", "foo"},
+			wantWords:    []string{"echo", "foo"},
 		},
 		{
-			name: "wordbreak prefix with equals",
-			input: "echo foo=bar",
-			format: BashFormat(),
-			wantWord: "foo=bar",
+			name:       "wordbreak prefix with equals",
+			input:      "echo foo=bar",
+			format:     BashFormat(),
+			wantWord:   "foo=bar",
 			wantPrefix: "foo=",
-			wantState: IN_WORD_STATE,
-			wantWords: []string{"echo", "foo=bar"},
+			wantState:  IN_WORD_STATE,
+			wantWords:  []string{"echo", "foo=bar"},
 		},
 		{
-			name: "empty input",
-			input: "",
-			format: BashFormat(),
-			wantWord: "",
+			name:       "empty input",
+			input:      "",
+			format:     BashFormat(),
+			wantWord:   "",
 			wantPrefix: "",
-			wantState: START_STATE,
-			wantWords: []string{""},
+			wantState:  START_STATE,
+			wantWords:  []string{""},
 		},
 		{
-			name: "escape at end",
-			input: `echo foo\`,
-			format: BashFormat(),
-			wantWord: "foo",
+			name:       "escape at end",
+			input:      `echo foo\`,
+			format:     BashFormat(),
+			wantWord:   "foo",
 			wantPrefix: "",
-			wantState: ESCAPING_STATE,
-			wantWords: []string{"echo", "foo"},
+			wantState:  ESCAPING_STATE,
+			wantWords:  []string{"echo", "foo"},
 		},
 	}
 
diff --git a/format_bash.go b/format_bash.go
@@ -42,4 +42,4 @@ func (bashFormat) KeywordOperators() map[string]WordbreakType { return nil }
 func (bashFormat) NonEscapingQuoteEscapes() bool { return false }
 
 func (bashFormat) NonEscapingQuoteBackslashEscapes() bool { return false }
-func (bashFormat) EscapeNotBareword() bool { return true }
+func (bashFormat) EscapeNotBareword() bool                { return true }
diff --git a/format_cmd.go b/format_cmd.go
@@ -62,6 +62,6 @@ func (cmdFormat) ClassifyOperator(raw string) WordbreakType {
 
 func (cmdFormat) KeywordOperators() map[string]WordbreakType { return nil }
 
-func (cmdFormat) NonEscapingQuoteEscapes() bool { return false }
+func (cmdFormat) NonEscapingQuoteEscapes() bool          { return false }
 func (cmdFormat) NonEscapingQuoteBackslashEscapes() bool { return false }
-func (cmdFormat) EscapeNotBareword() bool { return true } // ^ is always an escape
+func (cmdFormat) EscapeNotBareword() bool                { return true } // ^ is always an escape
diff --git a/format_elvish.go b/format_elvish.go
@@ -2,7 +2,7 @@ package shlex
 
 // elvishFormat implements Format for elvish lexing.
 // Key differences from bash:
-// - '' inside single quotes → literal ' (same as zsh RC_QUOTES)
+// - ” inside single quotes → literal ' (same as zsh RC_QUOTES)
 // - \ is NOT an escape character outside quotes (it's a bareword char)
 // - No POSIX list operators (no &&, ||, &)
 type elvishFormat struct{}
@@ -51,6 +51,6 @@ func (elvishFormat) ClassifyOperator(raw string) WordbreakType {
 
 func (elvishFormat) KeywordOperators() map[string]WordbreakType { return nil }
 
-func (elvishFormat) NonEscapingQuoteEscapes() bool { return true }  // '' → '
+func (elvishFormat) NonEscapingQuoteEscapes() bool          { return true } // '' → '
 func (elvishFormat) NonEscapingQuoteBackslashEscapes() bool { return false }
-func (elvishFormat) EscapeNotBareword() bool { return false } // \ is a bareword char in elvish
+func (elvishFormat) EscapeNotBareword() bool                { return false } // \ is a bareword char in elvish
diff --git a/format_fish.go b/format_fish.go
@@ -2,8 +2,9 @@ package shlex
 
 // fishFormat implements Format for fish lexing.
 // Key differences from bash:
-// - \' and \\ are escapes inside single quotes (NonEscapingQuoteEscapes)
-// - Keyword operators: and, or, not (bare words acting as operators)
+// - \' and \\ are escapes inside single quotes (NonEscapingQuoteBackslashEscapes)
+// - Keyword operators: and, or (bare words acting as pipeline delimiters)
+// - `not` is a prefix keyword but not a pipeline delimiter, so not in KeywordOperators
 // - No word splitting on variable expansion (doesn't affect lexing)
 // - Narrower escape set in double quotes (\" \$ \\ and \+newline only)
 type fishFormat struct{}
@@ -55,6 +56,6 @@ func (fishFormat) KeywordOperators() map[string]WordbreakType {
 	}
 }
 
-func (fishFormat) NonEscapingQuoteEscapes() bool { return true }  // ' and \\ inside single quotes
+func (fishFormat) NonEscapingQuoteEscapes() bool          { return true } // ' and \\ inside single quotes
 func (fishFormat) NonEscapingQuoteBackslashEscapes() bool { return true }
-func (fishFormat) EscapeNotBareword() bool { return true }
+func (fishFormat) EscapeNotBareword() bool                { return true }
diff --git a/format_nushell.go b/format_nushell.go
@@ -17,7 +17,7 @@ func NushellFormat() Format { return nushellFormat{} }
 func (nushellFormat) Classifier() tokenClassifier {
 	t := tokenClassifier{}
 	t.addRuneClass(spaceRunes, spaceRuneClass)
-	t.addRuneClass(escapingQuoteRunes, escapingQuoteRuneClass)     // "
+	t.addRuneClass(escapingQuoteRunes, escapingQuoteRuneClass)       // "
 	t.addRuneClass(nonEscapingQuoteRunes, nonEscapingQuoteRuneClass) // '
 	// Nushell: backtick is a quote character (not an escape)
 	t.addRuneClass("`", nonEscapingQuoteRuneClass)
@@ -54,6 +54,6 @@ func (nushellFormat) ClassifyOperator(raw string) WordbreakType {
 
 func (nushellFormat) KeywordOperators() map[string]WordbreakType { return nil }
 
-func (nushellFormat) NonEscapingQuoteEscapes() bool { return false }
+func (nushellFormat) NonEscapingQuoteEscapes() bool          { return false }
 func (nushellFormat) NonEscapingQuoteBackslashEscapes() bool { return false }
-func (nushellFormat) EscapeNotBareword() bool { return true }
+func (nushellFormat) EscapeNotBareword() bool                { return true }
diff --git a/format_oil.go b/format_oil.go
@@ -2,5 +2,5 @@ package shlex
 
 // OilFormat returns the Oil shell (OSH) lexical format.
 // OSH is bash-compatible, so it uses the bash format directly.
-// YSH string types (r'...', '''...''') are deferred to Phase 4.
+// YSH string types (r'...', ”'...”') are deferred to Phase 4.
 func OilFormat() Format { return bashFormat{} }
diff --git a/format_powershell.go b/format_powershell.go
@@ -2,12 +2,12 @@ package shlex
 
 // powershellFormat implements Format for PowerShell lexing.
 // Key differences from bash:
-// - Backtick (`) is the escape character, not backslash (\)
-// - '' inside single quotes → literal ' (doubled quote)
-// - "" inside double quotes → literal " (doubled quote)
-// - No single-quote-as-quote for outer quote pairs in the POSIX sense;
-//   both ' and " are quote chars
-// - Here-strings (@'...'@, @"..."@) and --% are deferred to Phase 4
+//   - Backtick (`) is the escape character, not backslash (\)
+//   - ” inside single quotes → literal ' (doubled quote)
+//   - "" inside double quotes → literal " (doubled quote)
+//   - No single-quote-as-quote for outer quote pairs in the POSIX sense;
+//     both ' and " are quote chars
+//   - Here-strings (@'...'@, @"..."@) and --% are deferred to Phase 4
 type powershellFormat struct{}
 
 // PowershellFormat returns the PowerShell lexical format.
@@ -16,7 +16,7 @@ func PowershellFormat() Format { return powershellFormat{} }
 func (powershellFormat) Classifier() tokenClassifier {
 	t := tokenClassifier{}
 	t.addRuneClass(spaceRunes, spaceRuneClass)
-	t.addRuneClass(escapingQuoteRunes, escapingQuoteRuneClass)  // " is escaping quote
+	t.addRuneClass(escapingQuoteRunes, escapingQuoteRuneClass)       // " is escaping quote
 	t.addRuneClass(nonEscapingQuoteRunes, nonEscapingQuoteRuneClass) // ' is non-escaping
 	// PowerShell: backtick is the escape character, not backslash
 	t.addRuneClass("`", escapeRuneClass)
@@ -58,6 +58,6 @@ func (powershellFormat) ClassifyOperator(raw string) WordbreakType {
 
 func (powershellFormat) KeywordOperators() map[string]WordbreakType { return nil }
 
-func (powershellFormat) NonEscapingQuoteEscapes() bool { return true }  // '' → '
+func (powershellFormat) NonEscapingQuoteEscapes() bool          { return true } // '' → '
 func (powershellFormat) NonEscapingQuoteBackslashEscapes() bool { return false }
-func (powershellFormat) EscapeNotBareword() bool { return true }
+func (powershellFormat) EscapeNotBareword() bool                { return true }
diff --git a/format_tcsh.go b/format_tcsh.go
@@ -20,6 +20,6 @@ func (tcshFormat) ClassifyOperator(raw string) WordbreakType {
 
 func (tcshFormat) KeywordOperators() map[string]WordbreakType { return nil }
 
-func (tcshFormat) NonEscapingQuoteEscapes() bool { return false }
+func (tcshFormat) NonEscapingQuoteEscapes() bool          { return false }
 func (tcshFormat) NonEscapingQuoteBackslashEscapes() bool { return false }
-func (tcshFormat) EscapeNotBareword() bool { return true }
+func (tcshFormat) EscapeNotBareword() bool                { return true }
diff --git a/format_xonsh.go b/format_xonsh.go
@@ -2,12 +2,12 @@ package shlex
 
 // xonshFormat implements Format for xonsh lexing.
 // Xonsh is a Python/shell hybrid. For lexing purposes:
-// - Standard single/double quotes work like bash
-// - Prefix strings (r'...', f'...', p'...', b'...') work because the
-//   prefix is a word char and Words() merges the segments
-// - Triple-quotes ('''...''', """...""""") need 3-rune lookahead (deferred)
-// - Shell operators: |, >, >>, <, ;, &&, ||, &
-// - \ is the escape char (Python rules inside quotes)
+//   - Standard single/double quotes work like bash
+//   - Prefix strings (r'...', f'...', p'...', b'...') work because the
+//     prefix is a word char and Words() merges the segments
+//   - Triple-quotes (”'...”', """...""""") need 3-rune lookahead (deferred)
+//   - Shell operators: |, >, >>, <, ;, &&, ||, &
+//   - \ is the escape char (Python rules inside quotes)
 type xonshFormat struct{}
 
 // XonshFormat returns the xonsh lexical format.
@@ -41,6 +41,6 @@ func (xonshFormat) ClassifyOperator(raw string) WordbreakType {
 
 func (xonshFormat) KeywordOperators() map[string]WordbreakType { return nil }
 
-func (xonshFormat) NonEscapingQuoteEscapes() bool { return false }
+func (xonshFormat) NonEscapingQuoteEscapes() bool          { return false }
 func (xonshFormat) NonEscapingQuoteBackslashEscapes() bool { return false }
-func (xonshFormat) EscapeNotBareword() bool { return true }
+func (xonshFormat) EscapeNotBareword() bool                { return true }
diff --git a/format_zsh.go b/format_zsh.go
@@ -1,15 +1,11 @@
 package shlex
 
 // zshFormat implements Format for zsh lexing.
-// Extends bash with RC_QUOTES ('' → ' inside single quotes) and
-// WORDCHARS-based wordbreak adjustment.
-type zshFormat struct {
-	wordchars string // characters that are NOT word breaks (from WORDCHARS env)
-}
+// Extends bash with RC_QUOTES (” → ' inside single quotes).
+type zshFormat struct{}
 
 // ZshFormat returns the zsh lexical format.
-// It reads WORDCHARS from the environment at Classifier() call time.
-// RC_QUOTES is enabled (zsh's default for '' inside single quotes).
+// RC_QUOTES is enabled (zsh's default for ” inside single quotes).
 func ZshFormat() Format { return zshFormat{} }
 
 func (zshFormat) Classifier() tokenClassifier {
@@ -22,6 +18,6 @@ func (zshFormat) ClassifyOperator(raw string) WordbreakType {
 
 func (zshFormat) KeywordOperators() map[string]WordbreakType { return nil }
 
-func (zshFormat) NonEscapingQuoteEscapes() bool { return true }  // RC_QUOTES: '' → '
+func (zshFormat) NonEscapingQuoteEscapes() bool          { return true } // RC_QUOTES: '' → '
 func (zshFormat) NonEscapingQuoteBackslashEscapes() bool { return false }
-func (zshFormat) EscapeNotBareword() bool { return true }
+func (zshFormat) EscapeNotBareword() bool                { return true }
diff --git a/plan.md b/plan.md
@@ -259,7 +259,7 @@ type TokenSlice []Token                    // unchanged operations
 type WordbreakType int                     // unchanged
 ```
 
-`TokenSlice` and `WordbreakType` remain. The `TokenSlice` helper methods (`CurrentPipeline`, `FilterRedirects`, `Words`, `WordbreakPrefix`, `CurrentToken`, `Strings`, `Pipelines`) become **unexported** — they're internal implementation details of `SplitForCompletion`. `Split` stays public for low-level token access (nushell `Patch()` uses `tokens[0].Value` directly, no helpers needed).
+`TokenSlice` and `WordbreakType` remain. The `TokenSlice` helper methods (`CurrentPipeline`, `FilterRedirects`, `Words`, `WordbreakPrefix`, `CurrentToken`, `Strings`, `Pipelines`) remain exported — the CLI and carapace both use them directly. `Split` stays public for low-level token access (nushell `Patch()` uses `tokens[0].Value` directly, no helpers needed).
 
 ---
 
@@ -357,7 +357,7 @@ Cmd needs the most format-specific logic: `REM` keyword-comment detection, `::`
 
 6. **`Span` rename of `Index`** — replacing `Token.Index` with `Token.Span.Start` is the one breaking change in Phase 1. It's mechanical (search/replace in tests) but touches the public API. **Alternative**: keep `Index` as a field and add `Span` alongside it, deprecating `Index` later. **Recommend**: do the clean rename in v2 since we're already breaking the internal `wordbreakType` API; carapace's usage of `Token.Index` is limited.
 
-7. **`CompletionContext` scope** — `SplitForCompletion` becomes the primary API for completion callers. The `TokenSlice` helper methods (`CurrentPipeline`, `FilterRedirects`, `Words`, `WordbreakPrefix`, `CurrentToken`) become internal implementation details. `Split` stays public for low-level use (nushell `Patch()`, raw token access). Carapace's `action.go`, `bash/patch.go`, `cmd_clink/patch.go`, and `zsh/action.go` migrate to `SplitForCompletion` — the zsh regex hack (4 regexes on `RawValue`) is replaced by `ctx.QuotingState`. `IsRedirect` is a field in `CompletionContext` (not caller logic), with `Pipeline TokenSlice` as the escape hatch for edge cases.
+7. **`CompletionContext` scope** — `SplitForCompletion` is the primary API for completion callers. The `TokenSlice` helper methods (`CurrentPipeline`, `FilterRedirects`, `Words`, `WordbreakPrefix`, `CurrentToken`) remain exported for the CLI and low-level use. `Split` stays public for low-level use (nushell `Patch()`, raw token access). Carapace's `action.go`, `bash/patch.go`, `cmd_clink/patch.go`, and `zsh/action.go` can migrate to `SplitForCompletion` — the zsh regex hack (4 regexes on `RawValue`) is replaced by `ctx.QuotingState`. `IsRedirect` is a field in `CompletionContext` (not caller logic), with `Pipeline TokenSlice` as the escape hatch for edge cases.
 
 ---
 
diff --git a/shlex.go b/shlex.go
@@ -25,7 +25,7 @@ func (l LexerState) MarshalJSON() ([]byte, error) {
 	return json.Marshal(lexerStates[l])
 }
 
-// Token is a (type, value) pair representing a lexographical token.
+// Token is a (type, value) pair representing a lexicographic token.
 type Token struct {
 	Type           TokenType
 	Value          string
@@ -139,7 +139,7 @@ func (typeMap tokenClassifier) addRuneClass(runes string, tokenType runeTokenCla
 	}
 }
 
-// ClassifyRune classifiees a rune
+// ClassifyRune classifies a rune
 func (t tokenClassifier) ClassifyRune(runeVal rune) runeTokenClass {
 	return t[runeVal]
 }
diff --git a/tokenslice.go b/tokenslice.go
@@ -78,7 +78,6 @@ func (t TokenSlice) FilterRedirects() TokenSlice {
 					}
 				}
 			}
-
 		}
 
 		filtered = append(filtered, token)
@@ -94,6 +93,9 @@ func (t TokenSlice) CurrentToken() (token Token) {
 }
 
 func (t TokenSlice) WordbreakPrefix() string {
+	if len(t) == 0 {
+		return ""
+	}
 	found := false
 	prefix := ""
 
diff --git a/wordbreak.go b/wordbreak.go

Original file line number	Diff line number	Diff line change
`@@ -84,8 +84,5 @@ func SplitForCompletionAt(s string, cursor int, format Format) *CompletionContex`
`84`	`84`	`}`
`85`	`85`
`86`	`86`	`ctx.Prefix = pipeline.WordbreakPrefix()`
`87`		`-`
`88`		`- _ = cursor // cursor position is implicitly at end (tokens already reflect full input)`
`89`		`- _ = err`
`90`	`87`	`return ctx`
`91`	`88`	`}`