search

chzyer / readline

Readline is a pure go(golang) implementation for GNU-Readline kind library
MIT License
2.08k stars 275 forks source link

remove Windows output translation #218

Closed slingamn closed 2 months ago

slingamn commented 1 year ago

According to https://github.com/jwalton/go-supportscolor the technique here is valid on all supported versions of Windows.

Without this change, 256-color ANSI codes will panic. For example, 48; sets a background color from the 256-color palette and causes an out-of-bounds read here:

https://github.com/chzyer/readline/blob/7f93d88cd5ffa0e805d58d2f9fc3191be15ec668/ansi_windows.go#L164

goroutine 14 [running]:
github.com/chzyer/readline.(*ANSIWriterCtx).ioloopEscSeq(0xc0000243c0?, 0xfe6142?, 0x6d, 0xc00001ef98)
        /redacted/ircdog/vendor/github.com/chzyer/readline/ansi_windows.go:164 +0x66f
github.com/chzyer/readline.(*ANSIWriterCtx).process(0xc00001ef90, 0x1b?)
        /redacted/ircdog/vendor/github.com/chzyer/readline/ansi_windows.go:99 +0xcf
github.com/chzyer/readline.(*ANSIWriter).Write(0xc00001efc0, {0xc000018640, 0xad, 0x140})
        /redacted/ircdog/vendor/github.com/chzyer/readline/ansi_windows.go:207 +0xdf
github.com/chzyer/readline.(*wrapWriter).Write.func1()
        /redacted/ircdog/vendor/github.com/chzyer/readline/operation.go:57 +0x44
github.com/chzyer/readline.(*RuneBuffer).Refresh(0xc000155cc0, 0xc00017bd40)
        /redacted/ircdog/vendor/github.com/chzyer/readline/runebuf.go:463 +0xd8
github.com/chzyer/readline.(*wrapWriter).Write(0xc00017bdb0, {0xc000018640?, 0xded668?, 0xc000021110?})
        /redacted/ircdog/vendor/github.com/chzyer/readline/operation.go:56 +0xa7
github.com/chzyer/readline.(*Instance).Write(0xc000021110?, {0xc000018640, 0xad, 0x140})
        /redacted/ircdog/vendor/github.com/chzyer/readline/readline.go:298 +0x85
fmt.Fprintln({0x1114660, 0xc000008378}, {0xc00017bef8, 0x1, 0x1})
        /usr/local/go/src/fmt/print.go:305 +0x75
main.connectExternal.func2()
        /redacted/ircdog/ircdog.go:303 +0x305
created by main.connectExternal
        /redacted/ircdog/ircdog.go:284 +0x2d9

Thanks very much for your time.

wader commented 1 year ago

Nice cleanup 👍 by "all supported versions of windows" you mean all version of windows supported by microsoft or golang?

slingamn commented 1 year ago

By Microsoft, sorry. Go still supports Windows 7: https://github.com/golang/go/issues/57003

wader commented 1 year ago

I see, thanks! some users of https://github.com/wader/fq seem to have some ANSI related issues on windows, for example underline with colored text somehow ends up changing the background color. I don't know much about windows or windows terminal so i'm bit lost :)

wader commented 1 year ago

btw fq uses this readline fork https://github.com/wader/readline that has a bunch of fixes, maybe interesting

wader commented 1 year ago

Did some testing with fq with these changes, seems to work fine. Do you know if ANSI colors are natively support in the "old" cmd.exe also? i tested using virtualbox and image from https://developer.microsoft.com/en-us/windows/downloads/virtual-machines/

slingamn commented 1 year ago

Yep, I tested with Windows 10 22H2 on metal in both PowerShell and cmd.exe. Both aspects of this patch (ANSI output rendering and input processing) are fully functional in both contexts.

FWIW I'm still working on contacting chzyer and putting together a fork.

wader commented 1 year ago

@slingamn Great 👍 let us know how it goes

NCLnclNCL commented 1 year ago

By Microsoft, sorry. Go still supports Windows 7: golang/go#57003

No, it end support

slingamn commented 1 year ago

@NCLnclNCL technically it is still supported as long as Go 1.20 is supported, i.e. until Go 1.22 is released :-)

slingamn commented 2 months ago

This is merged and in the stable release of https://github.com/ergochat/readline , closing the PR here