Skip to content

Commit

Permalink
fix: terminal clobbering (#142)
Browse files Browse the repository at this point in the history
Signed-off-by: Keith Zantow <kzantow@gmail.com>
  • Loading branch information
kzantow authored Sep 21, 2023
1 parent 741d29e commit 968adaa
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 4 deletions.
36 changes: 33 additions & 3 deletions cmd/quill/internal/ui/ui.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"os"
"strings"
"sync"
"time"

tea "github.com/charmbracelet/bubbletea"
"github.com/charmbracelet/lipgloss"
Expand Down Expand Up @@ -86,11 +87,26 @@ func (m *UI) Teardown(force bool) error {
if !force {
m.handler.State().Running.Wait()
m.program.Quit()
// typically in all cases we would want to wait for the UI to finish. However there are still error cases
// that are not accounted for, resulting in hangs. For now, we'll just wait for the UI to finish in the
// happy path only. There will always be an indication of the problem to the user via reporting the error
// string from the worker (outside of the UI after teardown).
m.running.Wait()
} else {
m.program.Kill()
}
_ = runWithTimeout(250*time.Millisecond, func() error {
m.handler.State().Running.Wait()
return nil
})

m.running.Wait()
// it may be tempting to use Kill() however it has been found that this can cause the terminal to be left in
// a bad state (where Ctrl+C and other control characters no longer works for future processes in that terminal).
m.program.Quit()

_ = runWithTimeout(250*time.Millisecond, func() error {
m.running.Wait()
return nil
})
}

// TODO: allow for writing out the full log output to the screen (only a partial log is shown currently)
// this needs coordination to know what the last frame event is to change the state accordingly (which isn't possible now)
Expand Down Expand Up @@ -200,3 +216,17 @@ func postUIEvents(quiet bool, events ...partybus.Event) {
}
}
}

func runWithTimeout(timeout time.Duration, fn func() error) (err error) {
c := make(chan struct{}, 1)
go func() {
err = fn()
c <- struct{}{}
}()
select {
case <-c:
case <-time.After(timeout):
return fmt.Errorf("timed out after %v", timeout)
}
return err
}
2 changes: 1 addition & 1 deletion internal/log/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
var log = discard.New()

func Set(l logger.Logger) {
// though quill the application will automatically have a redaction logger, library consumers may not be doing this.
// though the application will automatically have a redaction logger, library consumers may not be doing this.
// for this reason we additionally ensure there is a redaction logger configured for any logger passed. The
// source of truth for redaction values is still in the internal redact package. If the passed logger is already
// redacted, then this is a no-op.
Expand Down

0 comments on commit 968adaa

Please sign in to comment.