Skip to content

fix: separate title and message in progress bar [IDE-1067] #335

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix: separate title and message in progress bar [IDE-1067] #335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8bdf266
fix: seperate title and message in progress bars
rrama Apr 22, 2025
ea51da6
Merge remote-tracking branch 'origin/main' into fix/IDE-1067-ui-progr…
rrama Apr 22, 2025
899cd85
chore: generate mocks
rrama Apr 22, 2025
4268109
fix: don't export unnecessary structs
rrama Apr 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 28 additions & 19 deletions pkg/local_workflows/code_workflow/native_workflow.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,38 +53,50 @@ const (

type OptionalAnalysisFunctions func(string, func() *http.Client, *zerolog.Logger, configuration.Configuration, ui.UserInterface) (*sarif.SarifResponse, *scan.ResultMetaData, error)

type ProgressTrackerFactory struct {
func NewTrackerFactory(userInterface ui.UserInterface, logger *zerolog.Logger) scan.TrackerFactory {
return progressTrackerFactory{
userInterface: userInterface,
logger: logger,
}
}

type progressTrackerFactory struct {
userInterface ui.UserInterface
logger *zerolog.Logger
}

func (p ProgressTrackerFactory) GenerateTracker() scan.Tracker {
return &ProgressTrackerAdapter{
bar: p.userInterface.NewProgressBar(),
logger: p.logger,
func (p progressTrackerFactory) GenerateTracker() scan.Tracker {
return &progressTrackerAdapter{
userInterface: p.userInterface,
logger: p.logger,
}
}

type ProgressTrackerAdapter struct {
bar ui.ProgressBar
logger *zerolog.Logger
type progressTrackerAdapter struct {
userInterface ui.UserInterface
logger *zerolog.Logger
bar ui.ProgressBar
}

func (p ProgressTrackerAdapter) Begin(title, message string) {
if len(message) > 0 {
p.bar.SetTitle(title + " - " + message)
} else {
p.bar.SetTitle(title)
func (p *progressTrackerAdapter) Begin(title, message string) {
if p.bar != nil {
p.logger.Error().Msg("progress tracker already begun")
return
}
p.bar = p.userInterface.NewProgressBar(title)
p.bar.SetMessage(message)

err := p.bar.UpdateProgress(ui.InfiniteProgress)
if err != nil {
p.logger.Err(err).Msg("Failed to update progress")
}
}

func (p ProgressTrackerAdapter) End(message string) {
p.bar.SetTitle(message)
func (p *progressTrackerAdapter) End(message string) {
if p.bar == nil {
return
}
p.bar.SetMessage(message)
err := p.bar.Clear()
if err != nil {
p.logger.Err(err).Msg("Failed to clear progress")
Expand Down Expand Up @@ -204,10 +216,7 @@ func defaultAnalyzeFunction(path string, httpClientFunc func() *http.Client, log
localConfiguration: config,
}

progressFactory := ProgressTrackerFactory{
userInterface: userInterface,
logger: logger,
}
progressFactory := NewTrackerFactory(userInterface, logger)

analysisOptions := []codeclient.AnalysisOption{}

Expand Down
3 changes: 1 addition & 2 deletions pkg/local_workflows/data_transformation_workflow.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ func dataTransformationEntryPoint(invocationCtx workflow.InvocationContext, inpu
return output, nil
}

progress := invocationCtx.GetUserInterface().NewProgressBar()
progress.SetTitle("Transforming data")
progress := invocationCtx.GetUserInterface().NewProgressBar("Transforming data")
progressError := progress.UpdateProgress(ui.InfiniteProgress)
if progressError != nil {
logger.Err(progressError).Msgf("Error when setting progress")
Expand Down
12 changes: 6 additions & 6 deletions pkg/mocks/progressbar.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions pkg/mocks/userinterface.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 6 additions & 11 deletions pkg/ui/consoleui_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ func Test_ProgressBar_Spinner(t *testing.T) {
var err error
writer := &bytes.Buffer{}

bar := newProgressBar(writer, SpinnerType, false)
bar.SetTitle("Hello")
bar := newProgressBar(writer, "Hello", SpinnerType, false)

err = bar.UpdateProgress(0)
assert.NoError(t, err)
Expand All @@ -40,8 +39,7 @@ func Test_ProgressBar_Spinner_Infinite(t *testing.T) {
var err error
writer := &bytes.Buffer{}

bar := newProgressBar(writer, SpinnerType, false)
bar.SetTitle("Hello")
bar := newProgressBar(writer, "Hello", SpinnerType, false)

err = bar.UpdateProgress(InfiniteProgress)
assert.NoError(t, err)
Expand All @@ -64,8 +62,7 @@ func Test_ProgressBar_Bar(t *testing.T) {
var err error
writer := &bytes.Buffer{}

bar := newProgressBar(writer, BarType, false)
bar.SetTitle("Hello")
bar := newProgressBar(writer, "Hello", BarType, false)

err = bar.UpdateProgress(0)
assert.NoError(t, err)
Expand All @@ -88,8 +85,7 @@ func Test_ProgressBar_Unknown(t *testing.T) {
var err error
writer := &bytes.Buffer{}

bar := newProgressBar(writer, "Unknown", false)
bar.SetTitle("Hello")
bar := newProgressBar(writer, "Hello", "Unknown", false)

err = bar.UpdateProgress(0)
assert.NoError(t, err)
Expand Down Expand Up @@ -120,11 +116,10 @@ func Test_DefaultUi(t *testing.T) {
stdin.WriteString(name + "\n")

ui := newConsoleUi(stdin, stdout, stderr)
bar := ui.NewProgressBar()
bar := ui.NewProgressBar("A Title")
assert.NotNil(t, bar)

// the bar will not render since the writer is not a TTY
bar.SetTitle("Hello")
err := bar.UpdateProgress(InfiniteProgress)
assert.NoError(t, err)

Expand Down Expand Up @@ -161,7 +156,7 @@ func Test_OutputError(t *testing.T) {

t.Run("Error Catalog error", func(t *testing.T) {
err := snyk.NewBadRequestError("If you carry on like this you will ensure the wrath of OWASP, the God of Code Injection.")
err.Links = []string{"http://example.com/docs"}
err.Links = []string{"https://example.com/docs"}

lipgloss.SetColorProfile(termenv.TrueColor)
uiError := ui.OutputError(err)
Expand Down
43 changes: 28 additions & 15 deletions pkg/ui/progressbar.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,52 +29,58 @@ const (
// It is used to show the progress of some running task (or multiple).
// Example (Infinite Progress without a value):
//
// var pBar ProgressBar = ui.DefaultUi().NewProgressBar()
// pBar := ui.DefaultUi().NewProgressBar("CLI Downloader")
// defer pBar.Clear()
// pBar.SetTitle("Downloading...")
// pBar.SetMessage("Downloading...")
// _ = pBar.UpdateProgress(ui.InfiniteProgress)
//
// Example (with a value):
//
// var pBar ProgressBar = ui.DefaultUi().NewProgressBar()
// pBar := ui.DefaultUi().NewProgressBar("CLI Downloader")
// defer pBar.Clear()
// pBar.SetTitle("Downloading...")
// pBar.SetMessage("Downloading...")
// for i := 0; i <= 50; i++ {
// pBar.UpdateProgress(float64(i) / 100.0)
// time.Sleep(time.Millisecond * 50)
// }
//
// pBar.SetTitle("Installing...")
// pBar.SetMessage("Installing...")
// for i := 50; i <= 100; i++ {
// pBar.UpdateProgress(float64(i) / 100.0)
// time.Sleep(time.Millisecond * 50)
// }
//
// The title can be changed in the middle of the progress bar.
// The message can be changed in the middle of the progress bar.
// In some implementations no progress bar will be shown until the first UpdateProgress is called.
type ProgressBar interface {
// UpdateProgress updates the state of the progress bar.
// The argument `progress` should be a float64 between 0 and 1,
// where 0 represents 0% completion, and 1 represents 100% completion.
// Returns an error if the update operation fails.
UpdateProgress(progress float64) error

// SetTitle sets the title of the progress bar, which is displayed next to the bar.
// The title provides context or description for the operation that is being tracked.
SetTitle(title string)
// SetMessage sets the message of the progress bar, which is displayed next to the bar.
// The message is displayed alongside the title passed in by `NewProgressBar(title string)`.
// The message provides context or description for the operation that is being tracked.
// Set to "" to clear the message.
SetMessage(message string)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: @rrama did you integrate these changes in the CLI? I assume given that this PR changes interfaces, it'll require changes to existing extensions. Did you consider doing this without a breaking change?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PeterSchafer, for the CLI it requires merging the old SetTtitle into the NewProgressBar in cliv2/cmd/cliv2/debug.go and then for cli-extension-iac the same again in internal/commands/iactest/ui.go. I can make PRs ready to merge before this gets merged.

I had considered doing it without any knock on changes to the CLI, it would leave the interface being less intuitive to use, as the LS would have to ignore SetTitles after the first UpdateProgress is called. I am hoping this impact is minor enough, but if not then let me know and I can do a zero knock on changes.


// Clear removes the progress bar from the terminal.
// Clear removes the progress bar from the user interface.
// Returns an error if the clearing operation fails.
Clear() error
}

type emptyProgressBar struct{}

func (emptyProgressBar) UpdateProgress(float64) error { return nil }
func (emptyProgressBar) SetTitle(string) {}
func (emptyProgressBar) SetMessage(string) {}
func (emptyProgressBar) Clear() error { return nil }

func newProgressBar(writer io.Writer, t ProgressType, animated bool) *consoleProgressBar {
p := &consoleProgressBar{writer: writer}
func newProgressBar(writer io.Writer, title string, t ProgressType, animated bool) *consoleProgressBar {
p := &consoleProgressBar{
writer: writer,
title: title,
}
p.active.Store(true)
p.animationRunning = false
p.progressType = t
Expand All @@ -85,6 +91,7 @@ func newProgressBar(writer io.Writer, t ProgressType, animated bool) *consolePro
type consoleProgressBar struct {
writer io.Writer
title string
message string
state int
progress atomic.Pointer[float64]
active atomic.Bool
Expand All @@ -95,7 +102,7 @@ type consoleProgressBar struct {

func (p *consoleProgressBar) UpdateProgress(progress float64) error {
if !p.active.Load() {
return fmt.Errorf("progress not active")
return fmt.Errorf("progress bar not active")
}

p.progress.Store(&progress)
Expand All @@ -110,7 +117,7 @@ func (p *consoleProgressBar) UpdateProgress(progress float64) error {
return nil
}

func (p *consoleProgressBar) SetTitle(title string) { p.title = title }
func (p *consoleProgressBar) SetMessage(message string) { p.message = message }

func (p *consoleProgressBar) Clear() error {
if !p.active.Load() {
Expand Down Expand Up @@ -141,6 +148,12 @@ func (p *consoleProgressBar) update() {

if len(p.title) > 0 {
progressString += p.title
if len(p.message) > 0 {
progressString += " - "
}
}
if len(p.message) > 0 {
progressString += p.message
}

p.state++
Expand Down
12 changes: 6 additions & 6 deletions pkg/ui/userinterface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
type UserInterface interface {
Output(output string) error
OutputError(err error, opts ...Opts) error
NewProgressBar() ProgressBar
NewProgressBar(title string) ProgressBar
Input(prompt string) (string, error)
}

Expand All @@ -38,10 +38,10 @@ func newConsoleUi(in io.Reader, out io.Writer, err io.Writer) UserInterface {
reader: bufio.NewReader(in),
}

defaultUi.progressBarFactory = func() ProgressBar {
defaultUi.progressBarFactory = func(title string) ProgressBar {
if stderr, ok := err.(*os.File); ok {
if isatty.IsTerminal(stderr.Fd()) || isatty.IsCygwinTerminal(stderr.Fd()) {
return newProgressBar(err, SpinnerType, true)
return newProgressBar(err, title, SpinnerType, true)
}
}

Expand All @@ -54,7 +54,7 @@ func newConsoleUi(in io.Reader, out io.Writer, err io.Writer) UserInterface {
type consoleUi struct {
writer io.Writer
errorWriter io.Writer
progressBarFactory func() ProgressBar
progressBarFactory func(title string) ProgressBar
reader *bufio.Reader
}

Expand Down Expand Up @@ -102,8 +102,8 @@ func (ui *consoleUi) OutputError(err error, opts ...Opts) error {
return utils.ErrorOf(fmt.Fprintln(ui.errorWriter, err.Error()))
}

func (ui *consoleUi) NewProgressBar() ProgressBar {
return ui.progressBarFactory()
func (ui *consoleUi) NewProgressBar(title string) ProgressBar {
return ui.progressBarFactory(title)
}

func (ui *consoleUi) Input(prompt string) (string, error) {
Expand Down