Add completion testsuite covering bash, zsh, fish and tcsh - #670
Merged
Conversation
Add a 'completion' DejaGnu tool, alongside modules/install/lint, that
drives a real bash pty through Expect to press Tab against the built
init/bash_completion script and check the resulting candidate list,
instead of just calling the completion functions directly.
The completion_<shell>_{start,raw,list,inline,close} driving procs
live in their own numbered file (020-bash-procs.exp) separate from
the shell-agnostic assert helpers in 006-procs.exp, so a future shell
only needs its own procs + test file. A dedicated 010-environ.exp
sets up a clean, reproducible environment for every spawned session
(sandboxed $HOME, no pager, no color, ...). Wired into
'make testcompletion' and 'script/mt comp'. Documented in
doc/source/devel/testsuite.rst.
Covers 'module'/'ml' completion: avail/load/unload module name and
option flag listing and prefix filtering (including the ambiguous
common-prefix and unique-inline cases), the 'nospace' directory-style
completion path, use/unuse filesystem and modulepath completion, the
saved-collection completion of restore, and the extra '-<name>'
candidates ml lists for already-loaded modules.
Getting this reliable against a real pty needed a few infrastructure
fixes along the way: 'after 300' before reading an inline completion
so a trailing space in flight isn't raced past, widening the spawned
pty (stty rows 40 columns 500) so a completed line landing exactly on
the default 80-column width doesn't trigger a wrap-redraw that looks
like stray output, disabling the readline query-items threshold so a
large candidate list doesn't block on a "Display all N possibilities?"
prompt, and sandboxing $HOME so 'module save' never writes real
collection files outside the test tree.
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
Run the 'completion' DejaGnu tool (script/mt comp) on push and pull request, mirroring the shape of lint_tests.yaml but with the bash-only package set the completion.00-init tests actually need. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
module help's usage synopsis showed no options at all for several common sub-commands (load, unload, switch, display/show/test/path/ paths, is-avail, mod-to-sh), and an incomplete set for others (whatis and lint were both missing -i/--icase), even though the bash completion script already offers the full set for each. Add the missing brackets so the documented usage matches what completion actually proposes. Also document 'remove' next to 'rm | unload': it is a real, registered alias (tcl/main.tcl parseModuleCommandName), not just an abbreviation, and completion already offers it. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
initadd, initclear, initlist, initprepend and initrm are old, hidden init-family commands intentionally left out of module help's documented sub-command list (they still work if typed out in full, kept only for backward compatibility). Stop offering them as Tab completion candidates, in bash, fish and zsh, so the completion candidate list matches the public, documented command surface. tcsh shares the bash candidate list (@comp_cmds@ in init/Makefile), so it is already fixed by the same change. Also drop every remaining reference to them: the now-unreachable no-op case arm handling them as the previous word in bash, the tcsh "n/initadd/n/"-style specs, and the fish predicate functions/ completions that only existed to complete their own arguments. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
'del' is offered as a Tab completion candidate for unload, but it is just the 3-character minimum abbreviation of 'delete', a real, case-sensitive shortcut resolved by parseModuleCommandName the same way 'remove' is -- 'delete' is the name worth advertising, not the arbitrary minimum-length form of it. Add it next to rm/remove/unload in module help's alias listing. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
module use accepts real -a/--append and -p/--prepend options (parsed in tcl/main.tcl) and module help already documents 'use [-a|-p] dir [...]', but completion offered nothing for use: in bash falling through to plain filename completion unconditionally, in fish not offering them at all. Only offer the two options once the word being completed starts with '-' (bash), so directory completion still applies otherwise; fish gets them as top-level '--append'/'--prepend' entries. zsh already had both. tcsh cannot express this: its complete selectors are evaluated left-to-right with first-match-wins, so a second 'n/use/...' spec for the same previous word never fires once the earlier 'n/use/d/' directory-completion spec has already matched -- there is no selector combining "previous word is use" with "current word starts with -", so tcsh keeps plain directory completion only. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
Both flags are parsed generically for every sub-command (tcl/main.tcl), matching the global, always-available 'Options:' section rather than the sub-command-contextual 'Switches:' section they were listed under. Bash completion already only offers them at the bare, no-sub-command 'module -<Tab>' level, alongside the rest of Options, so this was simply filed under the wrong section. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
Their usage synopsis omitted -i/-o (avail, spider), -o (list) and every option at all (stashlist), even though bash completion already offers the full comp_avail_opts/comp_list_opts/comp_stashlist_opts set for each. Short forms only, matching the existing synopsis convention -- the short/long correspondence is already spelled out in the Switches section below. '-o' takes a LIST argument, per the '-o LIST | --output=LIST' Switches entry. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
…tion module help's Options section documents these three as always-available global flags (tcl/main.tcl parses them generically for any sub-command), but completion never offered them: bash/tcsh (shared @comp_opts@ in init/Makefile), fish and zsh. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
These path-family options (prepend-path, append-path, remove-path) were entirely undocumented in the Switches section, even though bash completion already offers them. Needed so the short/long correspondence for '-d' can be resolved unambiguously alongside the pre-existing avail/list '-d | --default' entry. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
prepend-path/append-path were missing --duplicates and remove-path was missing --index in their own usage synopsis, even though bash completion already offers them (comp_path_opts/comp_rm_path_opts in init/Makefile). Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
list and savelist both accept -S/--starts-with and -C/--contains (tcl/main.tcl passes search_match through to cmdModuleList/ cmdModuleSavelist, same as avail/spider), but completion never offered them, in bash/tcsh (shared comp_list_opts in init/Makefile) and zsh. fish already offers them, since its option completion is not scoped per sub-command. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
Parse module help's usage message into the documented sub-commands, global switches, and each sub-command's own option flags (completion.00-init/007-module_help.exp), then check that Tab completion proposes exactly that set, neither less nor more: a bare 'module <Tab>' against the sub-command/switch union, 'module -<Tab>' against the switches alone, and 'module <sub-command> -<Tab>' against that sub-command's own options (including sub-commands documented to take none at all). Per-sub-command option checks type '-' first rather than double-Tab from a bare trailing space: every option-bearing completion helper skips looking up real module/collection/modulepath candidates once the word starts with '-', isolating the option flags on their own, and it sidesteps the common-prefix auto-extension trap that a bare trailing space hits whenever a sub-command's entire candidate set already shares a leading '-' or '/' (e.g. plain 'aliases' or 'unuse' candidates). A sub-command with zero documented options still rings the bell readline sends for any Tab that finds zero completions. The short-form options in a sub-command's own usage line are resolved to their long form via the Switches section, which is also where a short flag reused with a different meaning on a specific sub-command (-a is --all almost everywhere but --append on 'use', -d is --default for avail/spider but --delim for the *-path commands) gets told apart, using the "(on 'x' sub-command)" annotation those entries carry. testcompletion never rebuilt $(MODULECMD) before running the testsuite, so the ground truth above could compare against a stale completion. Numbered 007 rather than right after the setup files it depends on (005/006), so that its ground-truth globals become part of the save_test_env baseline taken later at 011 -- and thus survive the reset_test_env call at the end of whichever shell's test file happens to run first, instead of being unset outright as newly-created state that was never part of that baseline. This matters once a second shell's test file needs the same ground truth after the first one's reset_test_env already ran. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
Classify each sub-command's positional argument kind from its own usage synopsis (module/collection/dir/stash/file/freeform), and check 'module <sub-command> <Tab><Tab>' proposes a real candidate of that kind, for the two kinds with predictable fixture data: an available/loaded module or a saved collection. 'list'/'savelist' are excluded: their argument is a search string filtering already-known state, not a name completion target, despite their usage synopsis documenting a module/collection-shaped argument. The per-sub-command dispatch of _ml (init/bash_completion.in) mirrors _module's case-for-case, so both the per-sub-command option ground truth and this new argument-kind ground truth apply identically to 'ml <sub-command>' -- mirrored here as their own sections rather than folded into the 'module' loops, so a fixture/mechanism difference between the two commands shows up as its own failure. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
savelist does not accept -o/--output (see the module help fix), but shared a single comp_list_opts candidate set with list, which does. Split it into a dedicated comp_savelist_opts (bash, tcsh via the shared Makefile variable, zsh); fish is unaffected, since its option completion is not scoped per sub-command in the first place. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
'del' was offered as the Tab completion candidate for unload across bash, fish, tcsh and zsh, but it is only the 3-character minimum abbreviation of 'delete' (see the shortcut matching done by parseModuleCommandName in tcl/main.tcl), now also documented as such in module help. Advertise 'delete' as the candidate/top-level completion entry instead, and drop the special-cased 'del' dispatch entries: it was only one arbitrary abbreviation length among several valid ones that all resolve the same way, so hard-coding just that one added no real value; a manually typed abbreviation still works, it just falls back to default argument completion instead of getting unload-specific candidates. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
Cross-checked the Module Sub-Commands section in module.rst against the reportUsage usage synopsis and the bash completion script: - 'remove' has been a real, registered alias of 'unload' since long before this rewrite (see NEWS.rst issue#200) and 'delete' is now advertised the same way, but neither ever got its own entry in module.rst; only 'rm' did. Add matching 'See unload' entries for both, alongside the existing 'rm' one. - 'use' accepts a real, functional -p/--prepend option (parsed in tcl/main.tcl) that was completely undocumented: the synopsis and prose only mentioned -a/--append. Document -p/--prepend next to it. Every other sub-command name and per-sub-command option already matches across the three; the legacy initadd/initclear/initlist/ initprepend/initrm/initswitch sub-commands and autoinit remain correctly absent from reportUsage and completion, since module help does not document them either. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
'module config' reports 72 configuration options, but Tab completion only offered 68 of them as candidates: csh_limit, ignored_dirs, siteconfig and tcl_ext_lib were missing. These four have no MODULES_* environment variable override (unlike every other option) and cannot be altered via 'module config name value', but they remain valid, real option names that 'module config name' can query, so they belong in the candidate list like the rest. Bash and Tcsh completion share the same comp_config_opts Makefile variable (init/Makefile), fixed in one place. Fish (init/fish_completion) and Zsh (init/zsh-functions/_module.in) each spell out their own copy of that same option list by hand instead, so they need the same four names added directly. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
The reportUsage 'config' line only mentioned '[--dump-state|name [val]]', omitting the '--reset name' form that module.rst already documents and that 'module config --reset name' really accepts. This also lets '--reset' resolve through the generic per-sub-command option parsing used by the completion ground-truth testsuite, the same way every other sub-command's flags do. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
…truth Fetch every existing configuration option name from a bare 'modulecmd bash config' execution, the same way 007-module_help.exp already does for 'module help', into a new 008-module_config.exp. Check it against 'config' sub-command completion two ways: its option-flag check folds into the same per-sub-command loop every other sub-command goes through (typing '--' instead of '-' first, since --dump-state/--reset are its only options and share that extra dash as a common prefix, which would otherwise eat one of the two Tabs the double-Tab listing relies on), and its configuration-option-name check folds into the loop that checks module/collection arguments, as a specific case alongside those. Neither loop needs 'config' to be skipped anymore. Numbered 008, right after 007-module_help.exp, for the same save_test_env-baseline reason that file is numbered ahead of 010/011 (see its own commit message) -- and add it to the mandatory completion setup file list in script/mt and testsuite.rst, an omission in the original addition of this file. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
The 'savelist' case in _module.in defines its own option list by hand (rather than sharing 'list' _arguments spec), and it was missing -a/--all: per module.rst, --all applies to 'savelist' the same way it does to avail/list/lint/search/spider/whatis, but this static list never picked it up. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
Add completion.00-init/030-zsh-procs.exp (a
completion_zsh_{start,raw,list,close} set, mirroring
020-bash-procs.exp) and 031-zsh.exp (mirroring 021-bash.exp
section-for-section), driving a real zsh pty through init/zsh-
functions/_module the same way the bash tests drive init/
bash_completion.
zle redraws candidates through a completely different byte stream
than readline, so the raw-output parsing is specific to zsh: no bell
is rung when a Tab silently extends the typed word to a longer, still-
ambiguous common prefix (only a Tab that cannot insert anything at all
rings the bell); AUTO_MENU inserts the first listed candidate inline
right after a listing; the default 2000-byte match_max Expect uses
silently truncates listings padded to a wide pty width, unless
increased; and a listing too tall for the pty scrolls it, leaving
neither a cursor-up marker nor the usual post-listing redraw to key
off of.
Two behavioral differences from bash are real, not test bugs, and the
assertions in 021-bash.exp are adjusted accordingly: zsh does not ring
that bell on a still-ambiguous common-prefix extension (see above, so
every such case drops the leading bell bash expects), and _module.in
offers modulefile/collection candidates and option flags as two
mutually exclusive sets gated on whether the word being completed
starts with '-', rather than the always-mixed-together comp_*_opts
bash uses, so a bare trailing-space listing only ever shows one set
at a time. _module.in also has no 'ml' support at all (only "compdef
module"), so every section mirroring 'ml' behavior in 021-bash.exp is
dropped here.
005-init_ts.exp gains the zsh binary/built-completion-script checks
already present there for bash, and the comp setup file list in
script/mt picks up 00/030 alongside the existing bash one.
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
reportUsage and bash/tcsh/zsh completion all advertise -a/--all for is-avail, but the option parser in tcl/main.tcl.in only treats -a as --all on a specific list of sub-commands (avail, aliases, search, whatis, ml, list, lint, savelist, spider) that does not include is-avail; for is-avail it falls through and is treated as a positional module-spec instead, which errors out (documented as intentional behavior in testsuite/modules.20-locate/ 113-hide-all-opt.exp: passing -a before the module name errors, passing --all after it is a silent no-op). Drop [-a] from the is-avail reportUsage line, drop -a/--all from the is-avail completion option list in init/Makefile (shared by bash and tcsh), and drop the matching -a/--all _arguments entry from the is-avail case in init/zsh-functions/_module.in, so the usage message and completions match what is-avail actually accepts. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
…hpop module.rst documents -f/--force for purge, reload (and its update alias), reset, restore, stash and stashpop, and the option already works for all of them: -f/--force is parsed unconditionally in the top-level switch in tcl/main.tcl.in regardless of sub-command, unlike -a/--all which is only recognized for a specific list of sub-commands. None of the six ever showed [-f] in reportUsage though, and none of them completed --force/-f in bash, tcsh or zsh. Add [-f] to each of their reportUsage lines in tcl/report.tcl.in, mirroring the existing clear [-f] line. Add a completion option list per sub-command to init/Makefile (shared by bash and tcsh), and wire purge/reload+update/reset/stash into their own completion case arms in init/bash_completion.in (both the module and ml functions) and init/tcsh_completion.in instead of the shared no-completion group they used to share with sub-commands that really take no option. Split restore and stashpop out of the collection-name/stash-name completion groups they shared with save/saveshow/describe/saverm/disable/ is-saved and stashshow/stashrm respectively, so they gain --force/-f alongside their existing name completion without wrongly adding it to the other, option-less sub-commands in those groups. Mirror the same split in init/zsh-functions/_module.in, plus new case arms for purge, reload/update, reset and stash. zsh and fish completion already describe -f/--force as "By-pass dependency consistency" throughout init/zsh-functions/_module.in and init/fish_completion (established well before the codespell pre-commit hook was added), and the new case arms above add several more occurrences of that same, consistent phrasing. codespell flags it as a typo for "Bypass" on every newly touched line, so add it to the codespell ignore-words-list rather than diverge these new completion entries from the wording already used everywhere else for this option. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
module.rst documents 'source [options] modulefile...' accepting --auto|--no-auto and -f|--force, both parsed unconditionally by tcl/main.tcl.in regardless of sub-command, but reportUsage showed no option at all for source, and it was bundled into the shared no-completion group in init/bash_completion.in and init/tcsh_completion.in alongside sub-commands that really take no option, so its options were never offered; zsh had no case arm for it whatsoever. Add [--auto|--no-auto] [-f] to the source reportUsage line in tcl/report.tcl.in, add a completion option list to init/Makefile (shared by bash and tcsh), and give source its own completion case arm in init/bash_completion.in (module and ml) and init/zsh-functions/_module.in. source takes a script file pathname, not a module name resolved against the modulepath, so unlike mod-to-sh its argument is left to plain filename completion rather than an avail-style candidate list: bash falls through to the shell's default file completion (the same 'case "$cur" in -*) ...; *) ;; esac' pattern already used for the directory argument of use), zsh calls the '_files' action, and tcsh uses its 'f' completion style (mirroring 'n/use/d/', which limits to directories instead since the argument of use is a directory). Fish already completes --auto/--no-auto/-f unconditionally after any sub-command, and already left source out of its avail-candidates function, so it already falls through to default file completion; no change needed there. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
The completion testsuite gained a zsh-driving counterpart to its original bash-only setup (030-zsh-procs.exp/031-zsh.exp, mirroring 020-bash-procs.exp/021-bash.exp), and 005-init_ts.exp/script/mt were updated accordingly at the time, but the testsuite documentation and NEWS.rst entry describing this testsuite still only mentioned bash. Update doc/source/devel/testsuite.rst to note both shells are covered, describe the zsh binary/built completion script set up alongside the bash one, and add 00/030 to the completion tool's mandatory setup file list documented for script/mt. Update the NEWS.rst entry introducing the completion testsuite to mention it drives a bash or zsh session. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
Add completion.00-init/040-fish-procs.exp (a
completion_fish_{start,raw,list,inline,close} set, mirroring
020-bash-procs.exp/030-zsh-procs.exp) and 041-fish.exp (mirroring
021-bash.exp/031-zsh.exp section-for-section), driving a real fish
pty through init/fish_completion the same way the bash/zsh tests
drive their own completion scripts.
The fish line editor differs from both readline and zle in ways that
shape almost every proc in 040-fish-procs.exp: on startup, a modern
fish queries the terminal for its capabilities and blocks waiting for
a reply -- up to a real 10 seconds for the primary device attributes
query alone -- so every proc that reads from the fish session answers
each such query the instant it appears, exactly as a real terminal
would (fish re-issues at least the cursor-position query on every
prompt redraw, not just once at startup). A single Tab is the fish
equivalent of the double-Tab bash and zsh both need: it extends the
typed word to the longest common prefix and shows the full candidate
pager in the same keypress, collapsing it back via one cursor-up
escape per pager row. fish echoes a command line back piecemeal as it
is typed, so recovering the final, settled state of a line means
taking the *last* place the typed text appears in the raw output, not
the first; the same applies to the listing the pager itself produces,
which needs enough idle time to fully stream out before it can be
captured without truncating a wide or tall one. Candidate names shown
with a description get rendered as "name (description text)",
requiring that parenthesized part to be stripped before splitting
into words, handling the one level of nested parentheses a
description can legitimately carry (e.g. "modulefile(s)").
Real behavioral differences from bash and zsh are documented in the
header of 041-fish.exp and reflected in its assertions rather than
worked around: init/fish_completion has no 'ml' support at all, does
not gate its option flags by sub-command (so every sub-command's
'-<Tab>' proposes the exact same full set), has no '-h'/'--help'
completion, and 'unuse' is not covered by any of its own predicates
(falling through to plain filename completion, unlike 'use').
completion_fish_start also neutralizes any *other* 'module' completion
fish can reach on the test machine once 'module' resolves to a real
command via autoinit: a system package (e.g. environment-modules on
Fedora, which installs its own, older completions/module.fish into
fish's vendor completions directory, a location this testsuite does
not control) can be loaded automatically by fish the first time
completion is attempted for 'module', clobbering the manual
registrations from 'source' with whatever that other version defines
instead. That automatic load only ever happens once per session, so
it is deliberately triggered and absorbed with a throwaway completion
attempt, its result erased, and the script under test re-sourced
fresh.
005-init_ts.exp gains the fish binary/built-completion-script checks
already present there for bash and zsh, and the comp setup file list
in script/mt picks up 00/040 alongside the existing bash/zsh ones.
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
The completion testsuite gained a fish-driving counterpart to its bash/zsh setup (040-fish-procs.exp/041-fish.exp, mirroring 020-bash-procs.exp/021-bash.exp and 030-zsh-procs.exp/031-zsh.exp), and 005-init_ts.exp/script/mt were updated accordingly at the time, but the testsuite documentation and NEWS.rst entry describing this testsuite still only mentioned bash and zsh. Update doc/source/devel/testsuite.rst to note all three shells are covered, describe the fish binary/built completion script set up alongside the bash and zsh ones, note that a shell whose completion script covers less ground documents each such gap in its own test file's header rather than forcing every file to test the same thing, and add 00/040 to the completion tool's mandatory setup file list documented for script/mt. Update the NEWS.rst entry introducing the completion testsuite to mention it drives a bash, zsh or fish session. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
The 'n/help/`_module_avail`/' rule in init/tcsh_completion.in only
ever offered available module names for 'module help <Tab>', unlike
the bash completion script, which groups 'help' together with
'show'/'display'/'test'/'path'/'paths'/'is-loaded'/'info-loaded' and
completes '-i'/'--icase' alongside the module names for all of them --
matching the usage synopsis of 'module help' itself ('help [-i]
[modulefile ...]').
Every one of those other sub-commands already carries this exact
"`_module_avail`; echo "@comp_mfile_opts@"" pattern in
init/tcsh_completion.in, except 'help', which was left calling
'_module_avail' bare. Complete the pattern so 'help' matches its
siblings and its own documented option.
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
…tsuite
Add completion.00-init/050-tcsh-procs.exp (a
completion_tcsh_{start,raw,list,inline,close} set, mirroring
020-bash-procs.exp/030-zsh-procs.exp/040-fish-procs.exp) and
051-tcsh.exp (mirroring 021-bash.exp section-for-section), driving a
real tcsh pty through the tcsh completion script the same way the
existing tests drive their own shell completion script.
The tcsh line editor differs from readline/zle/the fish line editor in
ways that shape almost every proc in 050-tcsh-procs.exp: Tab only ever
extends the typed word to the longest common prefix, ringing the bell
whenever the result is still ambiguous even when it did insert further
characters; listing every ambiguous match is bound to a separate key,
'^D', which never edits the line and always lists every candidate
matching the word exactly as typed (be there one, many, or none at
all) -- so, unlike the bash/zsh/fish double/single-Tab convention,
completion_tcsh_list sends no Tab at all, only '^D', sidestepping the
common-prefix auto-extension trap the other three shells work around
by choosing cmdline prefixes carefully. Every submitted command line
echoes back with a doubled '\r' before the '\n', and killing an input
line with '^U' redraws it via a cursor-column-position plus
clear-to-end-of-line escape rather than reprinting the prompt text.
init/tcsh_completion.in also does not shell out to the 'module'
command itself the way bash/zsh do to fetch candidates: its own
aliases invoke modulecmd.tcl directly through an absolute interpreter
path baked in at './configure' time, so 005-init_ts.exp rewrites that
one baked-in invocation to route through this test's own
$TCLSH/$MODULECMD before sourcing it, leaving the real 'complete
module ...'/'complete ml ...' registration under test untouched.
Real behavioral differences are documented in the header of
051-tcsh.exp and reflected in its assertions rather than worked
around: a unique "bar" -> "bar/" completion still rings a bell (tcsh
has no equivalent of the nospace exception that stays silent right at
a directory-style boundary); '^D' listings never collapse same-name
modules into one "name/" category candidate the way the other shells
own scripts do, showing the flat 'name/version' strings instead; 'ml'
has a far sparser registration than 'module' (no global switches at
all, and '-<Tab>' offers only the loaded-module unload shortcut, not
also the load option flags); 'unuse' only lists modulepaths from a
bare, empty word, since any '/'-prefixed word trips the unconditional
filename-completion rule declared ahead of the modulepath-specific
one; and 'use'/'source' each document an option flag or two that
init/tcsh_completion.in never wires into their own rule.
005-init_ts.exp gains the tcsh binary/built-completion-script checks
already present there for the other three shells, and the comp setup
file list in script/mt picks up 00/050 alongside the existing ones.
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
The completion testsuite gained a tcsh-driving counterpart to its
bash/zsh/fish setup (050-tcsh-procs.exp/051-tcsh.exp, mirroring
020-bash-procs.exp/030-zsh-procs.exp/040-fish-procs.exp), and
005-init_ts.exp/script/mt were updated accordingly at the time, but
the testsuite documentation and NEWS.rst entry describing this
testsuite still only mentioned bash, zsh and fish.
Update doc/source/devel/testsuite.rst to note all four shells are
covered, describe the tcsh binary/built completion script set up
alongside the other three, mention the two remaining sub-command gaps
noted in that file's own header ('ml' and 'unuse'), and add 00/050 to
the completion tool's mandatory setup file list documented for
script/mt. Update the NEWS.rst entry introducing the completion
testsuite to mention it drives a bash, zsh, fish or tcsh session.
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
005-init_ts.exp creates testsuite/completion-home once for the whole completion series and 010-environ.exp points $HOME at it for every completion_<shell>_start session afterwards, but nothing ever removed it again once the series finished, leaving it behind as stray build output after every completion test run. Add completion.00-init/999-cleanup.exp, matching the pattern already used by several modules series directories to delete on-disk fixtures a series created along the way (e.g. modules.99-finish, whose own 999-cleanup.exp is the closest precedent here: a final, no-continuation cleanup step). It has to run last, once nothing in the series still needs $HOME pointed there. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
completion_tcsh_list slept a blind, unconditional 1.5 real seconds before every single '^D' listing read, copied over from the zsh equivalent proc without re-checking whether tcsh actually needs it. zsh genuinely does: it has to send its cleanup keystroke blind, before reading anything back, since a listing taller than the pty has no reliable end marker of its own to match on, so a fixed settling delay is the only way to avoid corrupting the zle redraw. tcsh has no such constraint -- '^D' cannot print anything at all, not even a partial listing, until the whole candidate set is ready, so the 'expect' already right after it blocks for exactly as long as generating that candidate set genuinely takes, no fixed guess required. That blind sleep was multiplied by every per-sub-command ground-truth loop in 051-tcsh.exp -- 64 sub-commands, iterated over twice -- turning a handful of genuinely slow, child-process-backed calls into well over a hundred wasted seconds per run. Dropping it takes a full 'make testcompletion' run (all four shells) from roughly 380 real seconds to roughly 250, with no change in pass/fail results (737 passes, verified clean). A similar fixed-delay reduction was tried for the zsh equivalent wait and for the fish idle-quiet detection window, but both broke real assertions even at conservative values -- their designs commit to those constants for correctness, not just caution, so they are left untouched here. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| edit) _module_comgen_words_and_files "$(_module_avail "$cur")" "$cur";; | ||
| aliases) COMPREPLY=( $(compgen -W "@comp_aliases_opts@" -- "$cur") );; | ||
| list|savelist) COMPREPLY=( $(compgen -W "@comp_list_opts@" -- "$cur") );; | ||
| list) COMPREPLY=( $(compgen -W "@comp_list_opts@" -- "$cur") );; |
| aliases) COMPREPLY=( $(compgen -W "@comp_aliases_opts@" -- "$cur") );; | ||
| list|savelist) COMPREPLY=( $(compgen -W "@comp_list_opts@" -- "$cur") );; | ||
| list) COMPREPLY=( $(compgen -W "@comp_list_opts@" -- "$cur") );; | ||
| savelist) COMPREPLY=( $(compgen -W "@comp_savelist_opts@" -- "$cur") );; |
| savelist) COMPREPLY=( $(compgen -W "@comp_savelist_opts@" -- "$cur") );; | ||
| stashlist) COMPREPLY=( $(compgen -W "@comp_stashlist_opts@" -- "$cur") );; | ||
| stashpop|stashshow|stashrm) | ||
| stashpop) COMPREPLY=( $(compgen -W "@comp_stashpop_opts@ $(_module_stashlist)" -- "$cur") );; |
| COMPREPLY=( $(compgen -W "$(_module_stashlist)" -- "$cur") );; | ||
| clear) COMPREPLY=( $(compgen -W "@comp_clear_opts@" -- "$cur") );; | ||
| restore|save|saveshow|describe|saverm|disable|is-saved) | ||
| restore) COMPREPLY=( $(compgen -W "@comp_restore_opts@ $(_module_savelist)" -- "$cur") );; |
| unuse|is-used) COMPREPLY=( $(IFS=: compgen -W "${MODULEPATH}" -- "$cur") );; | ||
| use|-a|--append|cachebuild) ;; # let readline handle the completion | ||
| use) case "$cur" in | ||
| -*) COMPREPLY=( $(compgen -W "@comp_use_opts@" -- "$cur") );; |
| COMPREPLY=( $(compgen -W "@comp_search_opts@" -- "$cur") );; | ||
| config|--reset) COMPREPLY=( $(compgen -W "@comp_config_opts@" -- "$cur") );; | ||
| -h|--help|-V|--version|--dumpname|purge|refresh|reload|sh-to-mod|source|state|reset|stash|stashclear|cacheclear|update) | ||
| purge) COMPREPLY=( $(compgen -W "@comp_purge_opts@" -- "$cur") );; |
| config|--reset) COMPREPLY=( $(compgen -W "@comp_config_opts@" -- "$cur") );; | ||
| -h|--help|-V|--version|--dumpname|purge|refresh|reload|sh-to-mod|source|state|reset|stash|stashclear|cacheclear|update) | ||
| purge) COMPREPLY=( $(compgen -W "@comp_purge_opts@" -- "$cur") );; | ||
| reload|update) COMPREPLY=( $(compgen -W "@comp_reload_opts@" -- "$cur") );; |
| -h|--help|-V|--version|--dumpname|purge|refresh|reload|sh-to-mod|source|state|reset|stash|stashclear|cacheclear|update) | ||
| purge) COMPREPLY=( $(compgen -W "@comp_purge_opts@" -- "$cur") );; | ||
| reload|update) COMPREPLY=( $(compgen -W "@comp_reload_opts@" -- "$cur") );; | ||
| reset) COMPREPLY=( $(compgen -W "@comp_reset_opts@" -- "$cur") );; |
| purge) COMPREPLY=( $(compgen -W "@comp_purge_opts@" -- "$cur") );; | ||
| reload|update) COMPREPLY=( $(compgen -W "@comp_reload_opts@" -- "$cur") );; | ||
| reset) COMPREPLY=( $(compgen -W "@comp_reset_opts@" -- "$cur") );; | ||
| stash) COMPREPLY=( $(compgen -W "@comp_stash_opts@" -- "$cur") );; |
| reset) COMPREPLY=( $(compgen -W "@comp_reset_opts@" -- "$cur") );; | ||
| stash) COMPREPLY=( $(compgen -W "@comp_stash_opts@" -- "$cur") );; | ||
| source) case "$cur" in | ||
| -*) COMPREPLY=( $(compgen -W "@comp_source_opts@" -- "$cur") );; |
The package list in completion_tests.yaml was written back when the completion testsuite only drove bash, and never got updated as zsh, fish and tcsh support were added to it later on this same branch. The CI job (ubuntu-24.04) only had bash and Tcl installed, so every zsh/fish/tcsh session failed outright with 'couldn't execute "": no such file or directory' (an empty binary path, from the 'find_bin' lookup in 005-init_ts.exp coming up empty) as soon as 030-zsh-procs.exp, 040-fish-procs.exp and 050-tcsh-procs.exp tried to spawn one. Install zsh, fish and tcsh alongside bash so the job actually exercises every shell this testsuite is meant to cover. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
The end-of-listing detection in completion_zsh_list relies on a fixed escape-sequence epilogue (an attribute-reset sequence right before the redraw) that zle only emits when the pty actually has standout/ underline capability. On a pty without it (observed on GitHub Actions' own runners), the exact same auto-menu-selected redraw still happens -- a bare bell plus a plain erase-and-reprint, no escape sequences around it at all -- so the epilogue never matches, and the proc falls through to its "more than one line followed, must be a scrolled listing" branch, swallowing that trailing prompt-plus-cmdline redraw into the listing itself. The observed symptom on that CI job: a bare 'module <Tab><Tab>' listing whose real 64 sub-commands parsed out fine, but 'MODULESTEST> module' and a stray bell-prefixed word also got counted as "candidates", failing the ground-truth "no unknown word" check. Add a second, escape-sequence-independent detection: since that auto-menu-select redraw always re-echoes the prompt immediately followed by cmdline verbatim regardless of which escape sequences (if any) wrap it, search for that pair reappearing after the listing starts as a fallback listing-end boundary whenever the epilogue itself does not match. A genuinely scrolled listing (e.g. the same bare 'module', long enough to scroll a merely 40-row pty) still falls through unaffected, since zle never attempts that redraw once the current line has scrolled off screen -- there is nothing for this new check to find there either way. Verified against the exact raw byte sequence GitHub Actions' own log captured for the failing case (no live access to that exact zsh/pty combination from here), and against a full local 'make testcompletion' run (737 passes, 0 failures) to confirm the escape-sequence-epilogue path most environments hit is unaffected. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
completion_fish_wait_ready matched on the "prompt end" marker fish emits once a prompt has finished drawing -- but only on releases that support it under the running conditions. fish 3.7 (observed on a GitHub Actions runner, no recognized terminal-program hint) never emits it at all, no matter the prompt content, so the very first wait in completion_fish_start (before that CI job's own apt-get install of fish was even added) always timed out. That alone was survivable -- 'unresolved' does not abort the calling test -- but every setup command afterward went through completion_fish_raw, which relies on that exact same match, and since 'module' was consequently never actually sourced, every completion attempt for the rest of the file silently fell through to fish's own plain filename completion of the working directory (the observed symptom: real repository file names where module sub-command names were expected). Replace the marker matching entirely with the idle-quiet detection completion_fish_capture already relies on to read back a completion listing -- the same technique, applied to prompt waits instead of listings, needs no cooperation from fish itself and so works regardless of release or terminal-program recognition. Tried first: a version-independent fallback that also matched on the custom prompt own literal text once installed, alongside the marker -- abandoned because fish always appends cursor-positioning bytes right after printing a prompt, so a trailing-anchored text match never actually settles either. Verified with a full local 'make testcompletion' run (737 passes, 0 failures) and, before settling on this design, by deliberately breaking the marker pattern so it could never match, forcing every wait through a fallback path -- confirming the idle detection in completion_fish_capture (unaffected by any of this) was already carrying every real completion assertion throughout that run regardless. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: Xavier Delaruelle <xavier.delaruelle@cea.fr>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
completionDejaGnu testsuite (testsuite/completion.00-init,make testcompletion/script/mt comp) that drives a real interactive shell through Expect to press Tab against the built shell completion script and check the resulting candidate list, covering bash, zsh, fish and tcshcompletion_<shell>_{start,raw,list,inline,close}proc set plus a test file mirroring the others section-for-section, with genuine per-shell completion-script gaps documented and reflected in assertions rather than papered overmodule help/module configoutput rather than hardcoded expectations, so a documentation/completion-script drift shows up as a test failureis-avail/savelist/config/use/source/purge/reload/reset/stash/stashpop/tcsh'shelp, a staledelabbreviation, legacyinit*command leakage)$HOMEafter the series finishes