Skip to content

Commit

Permalink
fix: do not output ProgressMessage to non-interactive terminals (#23)
Browse files Browse the repository at this point in the history
This avoids including first progress update in the started message that is printed to non-interactive terminals.
  • Loading branch information
kangasta authored Nov 15, 2024
1 parent 8c6dcbb commit d5eaf82
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 26 deletions.
11 changes: 0 additions & 11 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,6 @@ linters:
# using `pkg/errors`, but those are generally moving towards stdlib.
- errorlint

# Checks that SQL statements are executed with the appropriate `.Exec` or
# `.Query` function.
- execinquery

# Checks that switches of enum(-like) types cover every possible case.
# Very helpful when we need to add new cases to enums, making sure that all
# the relevant bits are checked. Can become annoying if there's a _lot_ of
Expand Down Expand Up @@ -453,10 +449,6 @@ linters:
# problems.
- staticcheck

# Finds unused struct fields, i.e. fields that are not referenced from
# anywhere else. Helps remove dead code.
- structcheck

# Encourages various opinionated styles and rules including naming.
# Redundant with `revive`.
## - stylecheck # DISABLE
Expand Down Expand Up @@ -519,9 +511,6 @@ linters:
# Endorse use of various consts and variables defined in stdlib.
- usestdlibvars

# Finds unused global variables and constants. Helps remove dead code.
- varcheck

# Variable name length checks, takes definition and use span lengths into
# account. Nice idea, idiomatic Go, but needs discussion if we want to go
# with their defaults which cause a lot of errors in various existing
Expand Down
8 changes: 7 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,28 @@ The format is based on [keep a changelog](https://keepachangelog.com/en/1.0.0/)

## [Unreleased]

### Fixed

- Do not output `ProgressMessage` to non-interactive terminals. This avoids including a progress update in the started message that is printed to non-interactive terminals.

## [v1.0.2] - 2022-11-23

### Fixed

- Normalise whitespace (`\s`` `) in messages to avoid newlines and tabs breaking in-progress message updating.
- Assume details message to be preformatted, if it contains newline characters (`\n`). Preformatted message details are wrapped so that newline characters are maintained. This makes, for example, stack traces and console output in message details more readable.

## [v1.0.1] - 2022-08-30

### Fixed

- Do not try to render message if terminal width is zero. This happens with some terminals on first terminal width get(s).

## [v1.0.0] - 2022-08-26

### Added
- Extract and refactor livelog functionality from [UpCloud CLI (`upctl`)](https://github.com/UpCloudLtd/upcloud-cli.git).

- Extract and refactor livelog functionality from [UpCloud CLI (`upctl`)](https://github.com/UpCloudLtd/upcloud-cli.git).

[Unreleased]: https://github.com/UpCloudLtd/progress/compare/v1.0.2...HEAD
[v1.0.2]: https://github.com/UpCloudLtd/progress/compare/v1.0.1...v1.0.2
Expand Down
6 changes: 4 additions & 2 deletions messages/output.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,13 @@ func (cfg OutputConfig) formatDetails(msg *Message) string {
}

func (cfg OutputConfig) GetMessageText(msg *Message, renderState RenderState) string {
isInteractive := cfg.GetMaxHeight() > 0

status := ""
color := cfg.getStatusColor(msg.Status)
if cfg.ShowStatusIndicator {
indicator := cfg.getStatusIndicator(msg.Status)
if msg.Status.IsInProgress() && cfg.GetMaxHeight() > 0 {
if msg.Status.IsInProgress() && isInteractive {
indicator = cfg.getInProgressAnimationFrame(renderState)
}

Expand All @@ -208,7 +210,7 @@ func (cfg OutputConfig) GetMessageText(msg *Message, renderState RenderState) st

lenFn := text.RuneWidthWithoutEscSequences
message := msg.Message
if msg.ProgressMessage != "" {
if isInteractive && msg.ProgressMessage != "" {
message += " " + msg.ProgressMessage
}
maxMessageWidth := cfg.GetMaxWidth() - lenFn(status) - lenFn(elapsed)
Expand Down
46 changes: 34 additions & 12 deletions progress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ import (
"github.com/stretchr/testify/assert"
)

func removeColorsOnWindows(expected string) string {
if runtime.GOOS == "windows" {
re := regexp.MustCompile("\x1b\\[[0-9]+m")
return re.ReplaceAllString(expected, "")
}
return expected
}

func TestProgress_Push_ErrorChannel(t *testing.T) {
t.Parallel()
taskLog := progress.NewProgress(nil)
Expand Down Expand Up @@ -74,7 +82,7 @@ func TestProgress_Output(t *testing.T) {
err := taskLog.Push(messages.Update{Message: "Test update", Status: messages.MessageStatusStarted})
assert.NoError(t, err)

time.Sleep(time.Millisecond * 100) // Wait for the first render
time.Sleep(time.Millisecond * 150) // Wait for the first render

err = taskLog.Push(messages.Update{Message: "Test update", Status: messages.MessageStatusSuccess})
assert.NoError(t, err)
Expand All @@ -83,11 +91,29 @@ func TestProgress_Output(t *testing.T) {

output := buf.String()

expected := "\x1b[34m> \x1b[0mTest update \n\x1b[32m✓ \x1b[0mTest update \n"
if runtime.GOOS == "windows" {
re := regexp.MustCompile("\x1b\\[[0-9]+m")
expected = re.ReplaceAllString(expected, "")
}
expected := removeColorsOnWindows("\x1b[34m> \x1b[0mTest update \n\x1b[32m✓ \x1b[0mTest update \n")
assert.Equal(t, expected, output)
}

func TestProgress_NoProgressMessage(t *testing.T) {
t.Parallel()
cfg := progress.GetDefaultOutputConfig()
buf := bytes.NewBuffer(nil)
cfg.Target = buf

taskLog := progress.NewProgress(cfg)
taskLog.Start()
defer taskLog.Stop()

err := taskLog.Push(messages.Update{Message: "Test update", Status: messages.MessageStatusStarted})
assert.NoError(t, err)
err = taskLog.Push(messages.Update{Message: "Test update", ProgressMessage: "(50 %)"})
assert.NoError(t, err)

time.Sleep(time.Millisecond * 150) // Wait for the first render
output := buf.String()

expected := removeColorsOnWindows("\x1b[34m> \x1b[0mTest update \n")
assert.Equal(t, expected, output)
}

Expand All @@ -109,16 +135,12 @@ func TestProgress_ClosesInProgressMessagesOnStop(t *testing.T) {
err = taskLog.Push(messages.Update{Message: "Test pending 2", Status: messages.MessageStatusPending})
assert.NoError(t, err)

time.Sleep(time.Millisecond * 100) // Wait for the first render
time.Sleep(time.Millisecond * 150) // Wait for the first render

taskLog.Stop()

output := buf.String()

expected := "\x1b[34m> \x1b[0mTest started \n\x1b[35m- \x1b[0mTest pending 1 \n\x1b[35m- \x1b[0mTest pending 2 \n\x1b[37m? \x1b[0mTest started \n"
if runtime.GOOS == "windows" {
re := regexp.MustCompile("\x1b\\[[0-9]+m")
expected = re.ReplaceAllString(expected, "")
}
expected := removeColorsOnWindows("\x1b[34m> \x1b[0mTest started \n\x1b[35m- \x1b[0mTest pending 1 \n\x1b[35m- \x1b[0mTest pending 2 \n\x1b[37m? \x1b[0mTest started \n")
assert.Equal(t, expected, output)
}

0 comments on commit d5eaf82

Please sign in to comment.