Use ergochat/readline

[vishesh92]

Fixes #137

Migrates the CLI from the unmaintained github.com/chzyer/readline to
github.com/ergochat/readline v0.1.3.

vendor/ is retained

The original intent was to drop vendor/ as well, but the vendored readline
copy carries local changes that CloudMonkey depends on, and there is no way to
express them through the library's public API. vendor/ therefore stays
committed, and the local changes are now recorded rather than left implicit:

• patches/ergochat-readline-v0.1.3.patch — the local diff against upstream
  v0.1.3.
• patches/README.md — what each change does, why it exists, and its upstream
  status.
• make vendor-patch re-applies the patches after a go mod vendor;
  make vendor-check fails if they have gone missing.

The snap build no longer runs go mod vendor — it was regenerating the
committed tree and silently reverting the patches on every snap build — and now
builds explicitly with -mod=vendor so a missing vendor/ fails loudly
instead of quietly producing an unpatched binary.

Patches carried

1. Completion insert vs display (complete.go) — CloudMonkey builds
   candidates as <value> (<detail>) so a UUID is identifiable, but upstream's
   AutoCompleter.Do returns one slice used for both display and insertion.
   Forward-ported from the chzyer fixes for Fix autocompletion for API arguments #133 and autocomplete: prevent slice bounds panic #196.
2. Prompt redraw during completion (complete.go) — the ESC[J erase used
   while rendering the candidate grid could clear the prompt line. Upstream
   rendering bug, not CloudMonkey-specific.
3. Bounded cursor position query (operation.go) — upstream sends a DSR
   cursor query (ESC[6n) before every prompt and waits for the reply with no
   timeout, so under a pty that never answers (expect, pexpect, Ansible with a
   pty, some CI runners) cmk printed its banner and then hung with no prompt
   indefinitely. The query now has a 250ms deadline and the result is latched,
   so the cost is one timeout per session rather than one per prompt. Terminals
   that answer — including slow ones — are unaffected and keep full offset
   tracking. One-shot invocations (cmk listApis) never reach readline and were
   never affected.

None of the three are fixed in ergochat/readline v0.1.3, the latest release
since September 2024. Changes 2 and 3 are general bugs worth reporting upstream
so this patch set can shrink; change 1 needs an upstream API separating a
candidate's display text from its insert text.

Also included

• .github/workflows/ci.yml: use [mM]arvin-*.tar.gz for the Marvin install,
  matching apache/cloudstack's own workflow. Marvin's sdist filename case
  depends on the setuptools version in the runner (PEP 625 normalises to
  lowercase), and this job clones the latest CloudStack release tag. Also
  addressed independently in ci: fix marvin install failure #206.

[github-actions]

✅ Build complete for PR #199.

📦 Binary artifacts are available in the workflow run (expires on September 11, 2026).

Note: Download artifacts by clicking on the workflow run link above, then scroll to the "Artifacts" section.
Artifacts from PR builds are for testing only and may contain unreviewed, malicious code.

[Copilot]

Pull Request Overview

This PR migrates from the unmaintained github.com/chzyer/readline library to github.com/ergochat/readline and removes the vendor directory dependency management approach in favor of standard Go module handling.

• Replaces chzyer/readline with ergochat/readline v0.1.3
• Removes all vendor-related build flags and commands across build configuration files
• Updates dependency versions (golang.org/x/sys and adds golang.org/x/text)

Reviewed Changes

Copilot reviewed 4 out of 512 changed files in this pull request and generated no comments.

File	Description
go.mod	Updates readline dependency and bumps indirect dependencies
cli/prompt.go	Changes readline import to use ergochat package
Makefile	Removes -mod=vendor flags from all build commands and vendor dependencies from targets
snap/snapcraft.yaml	Removes go mod vendor command from snap build process

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[shwstppr]

@vishesh92 there were some customisations in the existing lib, not sure they need to be included with the change.
One I did was to allow autocompletion by name of the resources

[vishesh92]

@vishesh92 there were some customisations in the existing lib, not sure they need to be included with the change.
One I did was to allow autocompletion by name of the resources

Let me compare with the upstream readline library. Do you know if there is any other library which was changed? We should ideally move away from vendor based setup.

[vishesh92]

@shwstppr I have migrated the changes in readline package to the new one and keeping the vendors directory.

[shwstppr]

@vishesh92 tested build for this PR. For some reasons prompt goes missing when autocompletion is invoked
cmk-prompt-goes-missing.webm

[shwstppr]

LGTM

Not seeing any regressions due to library change
Good to have pagination when a lot of autocomplete options

[borisstoyanov]

Hi @vishesh92, I've tested this with claude and it came back with the following report. It passes feature parity, which is great and it indicates one point that it could potentially provoke a hang in the process. Could you please check if this is a valid concern.

Test results — PR #199 Use ergochat/readline

update-readline-and-remove-vendors @ 4041770 vs main @ de36f6d, built side by side on
darwin/arm64. Live CloudStack QA server, 905 APIs. Interactive checks driven through a real pty with a
VT emulator, screen + cursor diffed cell by cell.

Verdict

Functional parity with main	Yes — every check below matches
Improvement found	Emoji cursor handling is now correct
Issues found	2 (one blocking)

Issues

#	Severity	Issue
1	Blocking	ergochat sends ESC[6n before every prompt with no timeout (operation.go:422 passes a nil deadline to terminal.go:161). If the terminal never replies, cmk hangs with no prompt. main never sends it.
2	Minor	vendor/github.com/ergochat/readline/complete.go is hand-patched (the #133/#196 completion fixes). go mod vendor reverts 60 lines and silently breaks arg= completion. Nothing records the patch.

Issue 1, measured on a pty that ignores ESC[6n:

TERM	PR #199	main
xterm-256color	no prompt after 25 s	prompt in 0.04 s
dumb	no prompt after 6 s	prompt in 0.05 s
vt100	no prompt after 6 s	prompt in 0.05 s
unset	no prompt after 6 s	prompt in 0.05 s

With the reply supplied, PR reaches the prompt in 0.04 s. Real terminals all answer DSR, so the
exposure is pty automation — expect/pexpect, Ansible with a pty, some CI runners.

Improvement — emoji width

main patches the vendored runes.go to force emoji to width 1. ergochat uses x/text/width and
says 2, which is what terminals actually render. Prompt 🐱 > , input 🐱🐵, input starts at col 17:

Action	PR #199	main	Correct
type 🐱🐵🐱	23	23	23
type abc	20	20	20
Ctrl+A	17	19	17
one Left	19	20	19

Dropping the runes.go patch is right — ergochat fixes this properly upstream.

Non-interactive — 17 commands

Area	Result
version, -h, per-API help	identical
list zones in json / table / csv / column	identical
list zones -o text	key order differs — same binary differs run to run, pre-existing map ordering, not this PR
list domains, list accounts, list serviceofferings, list asyncjobs	identical
unknown API, invalid argument	identical
api listZones, -p override, exclude=	identical
sync against live server	905 APIs both, cache files byte-identical

Interactive — 28 pty scenarios

Area	Scenarios	Result
Completion	unique API, ambiguous prefix, arg list, arg prefix, id= live UUID values, verb prefix, no match	identical
Value completion	id=<8 chars>+Tab, full UUID+Tab, id=+Tab+Tab+Enter	identical
Editing	Ctrl+A/E/W/U/K/L, arrows, backspace, Ctrl+C, Ctrl+D, Ctrl+Z filtered	identical
Rendering	120-char wrap, CJK input, emoji input, 200-char paste, mid-line resize	identical
History	write to ~/.cmk/history, Up across restart, Ctrl+R search	identical
Live	sync, set output table, set profile, table render	identical
Emoji cursor	Ctrl+A / Left over emoji	PR correct, main off by 2

Build and deps

Check	Result
gofmt, go vet, go test ./...	clean
go mod verify	all modules verified
go mod tidy	no drift
go mod vendor	reverts the local patch — see issue 2
Cross-compile	OK on linux/amd64, linux/arm64, darwin/amd64, darwin/arm64, windows/amd64, freebsd/amd64
Binary size	14,146,082 → 14,200,258 bytes (+54 KB, +0.4 %)
Version currency	ergochat/readline v0.1.3 is the latest (Sep 2024)
Licensing	MIT + BSD-3, LICENSE and PATENTS vendored, .rat-excludes already covers vendor
Leftover refs	no chzyer references outside ergochat's own attribution line

Nit

Branch is named ...-and-remove-vendors, but vendor/ stays (396 files changed inside it).

[vishesh92]

to remove the vendors, we will need to maintain a fork of the readline library because we have some custom patches.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 401 changed files in this pull request and generated 4 comments.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.