peterh / liner

Pure Go line editor with history, inspired by linenoise
MIT License
1.04k stars 132 forks source link

speed up input #82

Closed tzneal closed 7 years ago

tzneal commented 8 years ago

There are two changes, one to enable some benchmarking and a second that speeds up counting glyphs.

I kept them separate as you may just want the second change without all of the kludging to allow tests to trick the library into thinking it's talking to a terminal instead of a pipe.

tzneal commented 8 years ago

I'd also appreciate your thoughts about making the function visible, something like:

func NewLinerWith(r io.Reader, w io.Writer, inFD int, outFD int) *State

to allow end to end unit tests by dependent libraries.

peterh commented 7 years ago

Sorry for the late reply.

I tried to pull, but it broke go test on Windows. If you'd rather not fix the benchmark on Windows, I can cherry-pick the "speed up input" commit. We can keep the benchmark in a separate branch.

Re: NewLinerWith. The main problem with NewLinerWith is the Windows input layer uses console handles instead of file descriptors, so even if you implement it on Windows, the function signature could change between uintptr and int. I don't mind if unexported functions have different signatures on different platforms, but I'd really rather not export a function that does not have a stable signature.

tzneal commented 7 years ago

No problem, I haven't had much of a chance to work on my personal project as of late anyway.

Go ahead and cherry pick the speed up commit, I wasn't happy with what I had to do to get the benchmark working anyway. I doubt there's much left in the way of low hanging fruit on the optimization front as well.

peterh commented 7 years ago

Okay, I've cherry-picked the speed-up commit into master, and cherry-picked the benchmark rework commit into bench (in case I need to reference it again).

Thanks! The benchmark rework was especially helpful when I was making more speed improvements.

tzneal commented 7 years ago

Great, glad to hear it!