readline icon indicating copy to clipboard operation
readline copied to clipboard

remove Windows output translation

Open slingamn opened this issue 1 year ago • 9 comments

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.

slingamn avatar Feb 08 '23 01:02 slingamn

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

wader avatar Feb 08 '23 09:02 wader

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

slingamn avatar Feb 08 '23 15:02 slingamn

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 avatar Feb 08 '23 15:02 wader

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

wader avatar Feb 08 '23 15:02 wader

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/

wader avatar Mar 06 '23 14:03 wader

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.

slingamn avatar Mar 06 '23 19:03 slingamn

@slingamn Great 👍 let us know how it goes

wader avatar Mar 07 '23 08:03 wader

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

No, it end support

NCLnclNCL avatar Jun 26 '23 18:06 NCLnclNCL

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

slingamn avatar Jun 30 '23 15:06 slingamn

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

slingamn avatar Jul 26 '24 06:07 slingamn