Remove duplicate github.com/vbauerster/mpb dependency#8991
Remove duplicate github.com/vbauerster/mpb dependency#8991macneale4 merged 1 commit intodolthub:mainfrom Juneezee:mpb
github.com/vbauerster/mpb dependency#8991Conversation
|
What's the motivation for removing this? Seems nice to have fewer dependencies, but the code is harder to read and maintain. |
go/cmd/dolt/cli/stdio_test.go
Outdated
| // clearLinesTxt moves cursor up n lines, and erases from cursor position to end of screen. | ||
| // https://github.com/vbauerster/mpb/blob/v8.0.2/cwriter/writer.go#L11-L15 | ||
| func clearLinesTxt(n int) string { | ||
| return fmt.Sprintf("%c[%dA", cwriter.ESC, n) + fmt.Sprintf("%c[J", cwriter.ESC) | ||
| return fmt.Sprintf("\x1b[%dA\x1b[J", n) |
There was a problem hiding this comment.
but the code is harder to read and maintain.
I added 2 lines of comment in case future readers find it hard to understand.
There was a problem hiding this comment.
yep, I saw that. But that package isn't the spec, so kind of an odd artifact. More appropriate would be this: https://en.wikipedia.org/wiki/ANSI_escape_code#C0_control_codes
But even given that, the code to start with is poor. What does "[%dA" do for example? I already needed to know/read the spec to figure that out. I'd be happy to take a change which makes it overall more readable. The change you proposed doesn't meet that requirement.
| return fmt.Sprintf("%c[%dA", cwriter.ESC, n) + fmt.Sprintf("%c[J", cwriter.ESC) | ||
| // These are ANSI escape codes, see | ||
| // - https://en.wikipedia.org/wiki/ANSI_escape_code#C0_control_codes1 | ||
| // - https://en.wikipedia.org/wiki/ANSI_escape_code#Control_Sequence_Introducer_commands | ||
| // | ||
| // \x1b: ESC (Escape) | ||
| // [%dA: CUU (Cursor Up). [5A means moves the cursor up 5 lines | ||
| // [J : ED (Erase in Display) | ||
| return fmt.Sprintf("\x1b[%dA\x1b[J", n) |
There was a problem hiding this comment.
@macneale4 I have added more comments. Hope you find this version easier to read.
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
|
Looks good to me. Gonna run though CI with a pr on my end |
3 participants
dolt/go/cmd/dolt/cli/stdio.go
Line 28 in b7c0ddb
dolt/go/cmd/dolt/cli/stdio_test.go
Line 23 in b7c0ddb
It's weird that we use the tagged
v8version in the source code but non-tagged version in the test.