usql icon indicating copy to clipboard operation
usql copied to clipboard

Replace unmaintained gohxs/readline with latest chzyer/readline

Open ottok opened this issue 5 months ago • 4 comments

The Go library gohxs/readline has no commits for past 8 years. It was originally forked from chzyer/readline, which continues to be maintained, so switch back to it to ensure continued maintenance and updates.

This change necessitated adapting the output filtering mechanism due to API incompatibilities that manifested as errors:

rline/rline.go:128:16: l.Inst.Config.Output undefined (type *readline.Config has no field or method Output)

rline/rline.go:130:7: cfg.FuncFilterOutput undefined (type *readline.Config has no field or method FuncFilterOutput)

Specifically, the readline.Config no longer provides direct output fields or filter functions. To address this, implement a filterWriter to wrap the standard output stream, ensuring existing output filtering functionality is preserved.

Closes: #528

ottok avatar Jul 26 '25 00:07 ottok

CI failed without error messages:

  go run testcli.go &> output.log
  ls -alh output.log
  shell: /usr/bin/bash -e {0}
Error: Process completed with exit code 1.

ottok avatar Jul 26 '25 00:07 ottok

I just tested this, it doesn't work with syntax highlighting, which basically makes this a no-go, as I personally made this project for 2 reasons: 1) to have a tool to use for all SQL databases with the xo/dbtpl project, and 2) syntax highlighting, as psql (and every other native SQL CLI I've used), doesn't support syntax highlighting.

kenshaw avatar Jul 26 '25 00:07 kenshaw

According to https://github.com/chzyer/readline/pull/116 the capability to syntax highlight was merged into Chyzer's version and that is probably also why the gohxs version seized to be maintained?

ottok avatar Jul 26 '25 00:07 ottok

Thanks @ccoVeille for the suggestion, but based on Ken's comment I think we need to have confirmation on the syntax highlightning issue https://github.com/chzyer/readline/issues/253 before it makes sense to polish the syntax on this change, as otherwise it won't be accepted anyway.

ottok avatar Jul 26 '25 17:07 ottok