From fc41d393946e6da59792da9ed6e5ab8ed6f58814 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Mon, 7 Dec 2015 16:04:42 -0800 Subject: [PATCH 1/2] Don't update lines on the terminal from a previous operation When we handle a message that isn't tracked in the "line" map (for example, one with no ID), clear the line map. This means we won't update lines that were part of a previous, completed set of operations when doing something like pull -a. It also has the beneficial side effect of avoiding terminal glitching in these types of situations, since messages that don't get tracked in the "line" map cause the count of the number of lines to get out of sync. Signed-off-by: Aaron Lehmann --- pkg/jsonmessage/jsonmessage.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pkg/jsonmessage/jsonmessage.go b/pkg/jsonmessage/jsonmessage.go index 451c6a9fd2..4f7a6fafc0 100644 --- a/pkg/jsonmessage/jsonmessage.go +++ b/pkg/jsonmessage/jsonmessage.go @@ -169,6 +169,12 @@ func DisplayJSONMessagesStream(in io.Reader, out io.Writer, terminalFd uintptr, if jm.ID != "" && (jm.Progress != nil || jm.ProgressMessage != "") { line, ok := ids[jm.ID] if !ok { + // NOTE: This approach of using len(id) to + // figure out the number of lines of history + // only works as long as we clear the history + // when we output something that's not + // accounted for in the map, such as a line + // with no ID. line = len(ids) ids[jm.ID] = line if isTerminal { @@ -182,6 +188,13 @@ func DisplayJSONMessagesStream(in io.Reader, out io.Writer, terminalFd uintptr, // [{diff}A = move cursor up diff rows fmt.Fprintf(out, "%c[%dA", 27, diff) } + } else { + // When outputting something that isn't progress + // output, clear the history of previous lines. We + // don't want progress entries from some previous + // operation to be updated (for example, pull -a + // with multiple tags). + ids = make(map[string]int) } err := jm.Display(out, isTerminal) if jm.ID != "" && isTerminal { From 59df2adc071e0186ccd0ba7ea9387142e168d735 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Mon, 7 Dec 2015 17:01:47 -0800 Subject: [PATCH 2/2] Fix the scoping of "diff" so its value doesn't leak between loop iterations In the existing code, "diff" has function scope and the value from the previous iteration may be used if it is not reset. This appears to be an oversight. This commit changes its scope to the for loop body. One confusing point is that the cursor movement escape sequences appear to be necessary even if the requested movement is 0. I haven't been able to figure out why this makes a difference. Signed-off-by: Aaron Lehmann --- pkg/jsonmessage/jsonmessage.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/jsonmessage/jsonmessage.go b/pkg/jsonmessage/jsonmessage.go index 4f7a6fafc0..f503b109f2 100644 --- a/pkg/jsonmessage/jsonmessage.go +++ b/pkg/jsonmessage/jsonmessage.go @@ -150,11 +150,11 @@ func (jm *JSONMessage) Display(out io.Writer, isTerminal bool) error { // each line and move the cursor while displaying. func DisplayJSONMessagesStream(in io.Reader, out io.Writer, terminalFd uintptr, isTerminal bool) error { var ( - dec = json.NewDecoder(in) - ids = make(map[string]int) - diff = 0 + dec = json.NewDecoder(in) + ids = make(map[string]int) ) for { + diff := 0 var jm JSONMessage if err := dec.Decode(&jm); err != nil { if err == io.EOF { @@ -180,11 +180,12 @@ func DisplayJSONMessagesStream(in io.Reader, out io.Writer, terminalFd uintptr, if isTerminal { fmt.Fprintf(out, "\n") } - diff = 0 } else { diff = len(ids) - line } if jm.ID != "" && isTerminal { + // NOTE: this appears to be necessary even if + // diff == 0. // [{diff}A = move cursor up diff rows fmt.Fprintf(out, "%c[%dA", 27, diff) } @@ -198,6 +199,8 @@ func DisplayJSONMessagesStream(in io.Reader, out io.Writer, terminalFd uintptr, } err := jm.Display(out, isTerminal) if jm.ID != "" && isTerminal { + // NOTE: this appears to be necessary even if + // diff == 0. // [{diff}B = move cursor down diff rows fmt.Fprintf(out, "%c[%dB", 27, diff) }