From 968adaa91523fed65fa5dfc761b68ca4f491b9be Mon Sep 17 00:00:00 2001
From: Keith Zantow <kzantow@gmail.com>
Date: Thu, 21 Sep 2023 09:40:33 -0400
Subject: [PATCH] fix: terminal clobbering (#142)

Signed-off-by: Keith Zantow <kzantow@gmail.com>
---
 cmd/quill/internal/ui/ui.go | 36 +++++++++++++++++++++++++++++++++---
 internal/log/log.go         |  2 +-
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/cmd/quill/internal/ui/ui.go b/cmd/quill/internal/ui/ui.go
index e0c41a1d..fe1edf0f 100644
--- a/cmd/quill/internal/ui/ui.go
+++ b/cmd/quill/internal/ui/ui.go
@@ -5,6 +5,7 @@ import (
 	"os"
 	"strings"
 	"sync"
+	"time"
 
 	tea "github.com/charmbracelet/bubbletea"
 	"github.com/charmbracelet/lipgloss"
@@ -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)
@@ -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
+}
diff --git a/internal/log/log.go b/internal/log/log.go
index 6c5e27ed..4fdd4ebd 100644
--- a/internal/log/log.go
+++ b/internal/log/log.go
@@ -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.