Skip to content

Commit

Permalink
feat(tui): improve logging and error handling in setup
Browse files Browse the repository at this point in the history
- Set up stderr logger by default, allowing subcommands to override.
- Change TuiCommand to TUICommand for consistency.
- Enhance error messages in flag parsing and TUI setup.
- Refactor NewController to return error instead of using fatal logs.
- Implement detailed logging and error handling in TUI Execute method.
- Add unpublished log entries catch-up mechanism if TUI setup fails.
- Update comments and method documentation for clarity.

This fixes some weirdness around stderr messages popping into the
TUI (fixes in the sense that we follow a different strategy for
getting the messages to the user now).
  • Loading branch information
ja-he committed Jul 2, 2024
1 parent ea62bf9 commit b7aea34
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 44 deletions.
8 changes: 7 additions & 1 deletion cmd/dayplan.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,18 @@ import (
"os"

"github.com/jessevdk/go-flags"
"github.com/rs/zerolog"
"github.com/rs/zerolog/log"

"github.com/ja-he/dayplan/internal/control/cli"
)

// MAIN
func main() {
// set up stderr logger by default, subcommands (such as tui) may choose to
// change this
log.Logger = log.Output(zerolog.ConsoleWriter{Out: os.Stderr})

// parse the flags
parser := flags.NewParser(&cli.Opts, flags.Default)
parser.SubcommandsOptional = false
Expand All @@ -19,7 +25,7 @@ func main() {
if flags.WroteHelp(err) {
os.Exit(0)
} else if err != nil {
fmt.Fprintf(os.Stderr, "flag parsing error:\n > %s\n", err.Error())
fmt.Fprintf(os.Stderr, "fatal error (e.g. flag parsing):\n > %s\n", err.Error())
os.Exit(1)
}

Expand Down
2 changes: 1 addition & 1 deletion internal/control/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package cli
type CommandLineOpts struct {
Version bool `short:"v" long:"version" description:"Show the program version"`

TuiCommand TuiCommand `command:"tui" subcommands-optional:"true"`
TuiCommand TUICommand `command:"tui" subcommands-optional:"true"`
SummarizeCommand SummarizeCommand `command:"summarize" subcommands-optional:"true"`
TimesheetCommand TimesheetCommand `command:"timesheet" subcommands-optional:"true"`
AddCommand AddCommand `command:"add" subcommands-optional:"true"`
Expand Down
51 changes: 25 additions & 26 deletions internal/control/cli/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"sync"
"time"

"github.com/rs/zerolog"
"github.com/rs/zerolog/log"

"github.com/ja-he/dayplan/internal/control"
Expand All @@ -31,7 +30,7 @@ import (
)

// TODO: this absolutely does not belong here
func (t *Controller) GetDayFromFileHandler(date model.Date) *model.Day {
func (t *Controller) getDayFromFileHandler(date model.Date) *model.Day {
t.fhMutex.RLock()
fh, ok := t.FileHandlers[date]
t.fhMutex.RUnlock()
Expand All @@ -48,7 +47,7 @@ func (t *Controller) GetDayFromFileHandler(date model.Date) *model.Day {
}
}

// Controller
// Controller is the struct for the TUI controller.
type Controller struct {
data *control.ControlData
rootPane *panes.RootPane
Expand All @@ -73,15 +72,13 @@ type Controller struct {
syncer tui.ScreenSynchronizer
}

// NewController
// NewController creates a new Controller.
func NewController(
date model.Date,
envData control.EnvData,
categoryStyling styling.CategoryStyling,
stylesheet styling.Stylesheet,
stderrLogger zerolog.Logger,
tuiLogger zerolog.Logger,
) *Controller {
) (*Controller, error) {
controller := Controller{}

inputConfig := input.InputConfig{
Expand Down Expand Up @@ -153,10 +150,11 @@ func NewController(
return model.BacklogFromReader(backlogReader, categoryGetter)
}()
if err != nil {
tuiLogger.Error().Err(err).Str("file", backlogFilePath).Msg("could not read backlog")
return nil, fmt.Errorf("could not read backlog at '%s' (%w)", backlogFilePath, err)
} else {
tuiLogger.Info().Str("file", backlogFilePath).Msg("successfully read backlog")
log.Info().Str("file", backlogFilePath).Msg("successfully read backlog")
}
log.Info().Msg("just testing because this should be just dandy")

tasksWidth := 40
toolsWidth := func() int {
Expand Down Expand Up @@ -270,7 +268,7 @@ func NewController(
}
weekdayPaneInputTree, err := input.ConstructInputTree(eventsViewBaseInputMap)
if err != nil {
stderrLogger.Fatal().Err(err).Msg("could not construct weekday pane input tree")
return nil, fmt.Errorf("could not construct weekday pane input tree (%w)", err)
}
weekdayPane := func(dayIndex int) *panes.EventsPane {
return panes.NewEventsPane(
Expand Down Expand Up @@ -303,7 +301,7 @@ func NewController(
}
monthdayPaneInputTree, err := input.ConstructInputTree(eventsViewBaseInputMap)
if err != nil {
stderrLogger.Fatal().Err(err).Msg("could not construct monthday pane input tree")
return nil, fmt.Errorf("could not construct monthday pane input tree (%w)", err)
}
monthdayPane := func(dayIndex int) ui.Pane {
return panes.NewMaybePane(
Expand Down Expand Up @@ -671,7 +669,7 @@ func NewController(
},
)
if err != nil {
stderrLogger.Fatal().Err(err).Msg("failed to construct input tree for tasks pane")
return nil, fmt.Errorf("failed to construct input tree for tasks pane (%w)", err)
}
toolsInputTree, err := input.ConstructInputTree(
map[input.Keyspec]action.Action{
Expand Down Expand Up @@ -702,7 +700,7 @@ func NewController(
},
)
if err != nil {
stderrLogger.Fatal().Err(err).Msg("failed to construct input tree for tools pane")
return nil, fmt.Errorf("failed to construct input tree for tools pane (%w)", err)
}

// TODO(ja-he): move elsewhere
Expand Down Expand Up @@ -871,7 +869,7 @@ func NewController(
}
dayViewEventsPaneInputTree, err := input.ConstructInputTree(eventsPaneDayInputMap)
if err != nil {
stderrLogger.Fatal().Err(err).Msg("failed to construct input tree for day view pane's events subpane")
return nil, fmt.Errorf("failed to construct input tree for day view pane's events subpane (%w)", err)
}

tasksVisible := false
Expand Down Expand Up @@ -1132,7 +1130,8 @@ func NewController(
},
)
if err != nil {
stderrLogger.Fatal().Err(err).Msg("failed to construct input tree for event pane's resize mode")
log.Error().Err(err).Msg("failed to construct input tree for event pane's resize mode (this should really never happen)")
return
}
dayEventsPane.ApplyModalOverlay(input.CapturingOverlayWrap(eventResizeOverlay))
controller.data.EventEditMode = edit.EventEditModeResize
Expand All @@ -1152,7 +1151,7 @@ func NewController(
},
)
if err != nil {
stderrLogger.Fatal().Err(err).Msg("failed to construct input tree for root pane")
return nil, fmt.Errorf("failed to construct input tree for root pane (%w)", err)
}
var ensureDayViewMainPaneFocusIsOnVisible func()
updateMainPaneRightFlexWidth := func() {
Expand Down Expand Up @@ -1190,12 +1189,12 @@ func NewController(
},
)
if err != nil {
stderrLogger.Fatal().Err(err).Msg("failed to construct input tree for day view pane")
return nil, fmt.Errorf("failed to construct input tree for day view pane (%w)", err)
}

dayViewScrollablePaneInputTree, err := input.ConstructInputTree(scrollableZoomableInputMap)
if err != nil {
stderrLogger.Fatal().Err(err).Msg("failed to construct input tree for day view scrollable pane")
return nil, fmt.Errorf("failed to construct input tree for day view scrollable pane (%w)", err)
}
dayViewScrollablePane := panes.NewWrapperPane(
[]ui.Pane{
Expand Down Expand Up @@ -1233,7 +1232,7 @@ func NewController(
multidayViewEventsWrapperInputMap["l"] = action.NewSimple(func() string { return "go to next day" }, controller.goToNextDay)
weekViewEventsWrapperInputTree, err := input.ConstructInputTree(multidayViewEventsWrapperInputMap)
if err != nil {
stderrLogger.Fatal().Err(err).Msg("failed to construct input tree for multi-day wrapper pane")
return nil, fmt.Errorf("failed to construct input tree for multi-day wrapper pane (%w)", err)
}
weekViewEventsWrapper := panes.NewWrapperPane(
weekViewEventsPanes,
Expand All @@ -1242,7 +1241,7 @@ func NewController(
)
monthViewEventsWrapperInputTree, err := input.ConstructInputTree(multidayViewEventsWrapperInputMap)
if err != nil {
stderrLogger.Fatal().Err(err).Msg("failed to construct input tree for month view wrapper pane")
return nil, fmt.Errorf("failed to construct input tree for month view wrapper pane (%w)", err)
}
monthViewEventsWrapper := panes.NewWrapperPane(
monthViewEventsPanes,
Expand All @@ -1267,7 +1266,7 @@ func NewController(
ensureDayViewMainPaneFocusIsOnVisible = dayViewMainPane.EnsureFocusIsOnVisible
weekViewMainPaneInputTree, err := input.ConstructInputTree(map[input.Keyspec]action.Action{})
if err != nil {
stderrLogger.Fatal().Err(err).Msg("failed to construct input tree for week view main pane")
return nil, fmt.Errorf("failed to construct input tree for week view main pane (%w)", err)
}
weekViewMainPane := panes.NewWrapperPane(
[]ui.Pane{
Expand All @@ -1289,7 +1288,7 @@ func NewController(
)
monthViewMainPaneInputTree, err := input.ConstructInputTree(scrollableZoomableInputMap)
if err != nil {
stderrLogger.Fatal().Err(err).Msg("failed to construct input tree for month view main pane")
return nil, fmt.Errorf("failed to construct input tree for month view main pane (%w)", err)
}
monthViewMainPane := panes.NewWrapperPane(
[]ui.Pane{
Expand Down Expand Up @@ -1340,7 +1339,7 @@ func NewController(
}),
})
if err != nil {
stderrLogger.Fatal().Err(err).Msg("failed to construct input tree for summary pane")
return nil, fmt.Errorf("failed to construct input tree for summary pane (%w)", err)
}

helpPaneInputTree, err := input.ConstructInputTree(
Expand All @@ -1351,7 +1350,7 @@ func NewController(
},
)
if err != nil {
stderrLogger.Fatal().Err(err).Msg("failed to construct input tree for help pane")
return nil, fmt.Errorf("failed to construct input tree for help pane (%w)", err)
}
helpPane := panes.NewHelpPane(
ui.NewConstrainedRenderer(renderer, helpDimensions),
Expand Down Expand Up @@ -1521,7 +1520,7 @@ func NewController(

controller.data.MouseEditState = edit.MouseEditStateNone

return &controller
return &controller, nil
}

func (t *Controller) ScrollUp(by int) {
Expand Down Expand Up @@ -1628,7 +1627,7 @@ func (t *Controller) goToNextDay() {
func (t *Controller) loadDay(date model.Date) {
if !t.data.Days.HasDay(date) {
// load file
newDay := t.GetDayFromFileHandler(date)
newDay := t.getDayFromFileHandler(date)
if newDay == nil {
panic("newDay nil?!")
}
Expand Down
80 changes: 64 additions & 16 deletions internal/control/cli/tui.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cli

import (
"fmt"
"io"
"os"
"strings"
Expand All @@ -16,24 +17,23 @@ import (
"github.com/ja-he/dayplan/internal/styling"
)

type TuiCommand struct {
// TUICommand is the struct for the TUI command.
type TUICommand struct {
Day string `short:"d" long:"day" description:"Specify the day to plan" value-name:"<file>"`
Theme string `short:"t" long:"theme" choice:"light" choice:"dark" description:"Select a 'dark' or a 'light' default theme (note: only sets defaults, which are individually overridden by settings in config.yaml"`
LogOutputFile string `short:"l" long:"log-output-file" description:"specify a log output file (otherwise logs dropped)"`
LogPretty bool `short:"p" long:"log-pretty" description:"prettify logs to file"`
}

func (command *TuiCommand) Execute(args []string) error {
// set up stderr logger until TUI set up
stderrLogger := log.Output(zerolog.ConsoleWriter{Out: os.Stderr})

// Execute runs the TUI command.
func (command *TUICommand) Execute(_ []string) error {
// create TUI logger
var logWriter io.Writer
if command.LogOutputFile != "" {
var fileLogger io.Writer
file, err := os.OpenFile(command.LogOutputFile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
if err != nil {
stderrLogger.Fatal().Err(err).Str("file", command.LogOutputFile).Msg("could not open file for logging")
return fmt.Errorf("could not open file '%s' for logging (%w)", command.LogOutputFile, err)
}
if command.LogPretty {
fileLogger = zerolog.ConsoleWriter{Out: file}
Expand All @@ -46,10 +46,6 @@ func (command *TuiCommand) Execute(args []string) error {
}
tuiLogger := zerolog.New(logWriter).With().Timestamp().Caller().Logger()

// temporarily log to both (in case the TUI doesn't get set we want the info
// on the stderr logger, otherwise the TUI logger is relevant)
log.Logger = log.Output(zerolog.MultiLevelWriter(stderrLogger, tuiLogger))

var theme config.ColorschemeType
switch command.Theme {
case "light":
Expand Down Expand Up @@ -79,7 +75,7 @@ func (command *TuiCommand) Execute(args []string) error {
} else {
initialDay, err = model.FromString(command.Day)
if err != nil {
stderrLogger.Fatal().Err(err).Msg("could not parse given date") // TODO
return fmt.Errorf("could not parse given date (%w)", err)
}
}

Expand All @@ -96,12 +92,11 @@ func (command *TuiCommand) Execute(args []string) error {
}
configData, err := config.ParseConfigAugmentDefaults(theme, yamlData)
if err != nil {
stderrLogger.Fatal().Err(err).Msg("can't parse config data")
return fmt.Errorf("can't parse config data (%w)", err)
}

// get categories from config
var categoryStyling styling.CategoryStyling
categoryStyling = *styling.EmptyCategoryStyling()
categoryStyling := *styling.EmptyCategoryStyling()
for _, category := range configData.Categories {

var goal model.Goal
Expand All @@ -128,11 +123,64 @@ func (command *TuiCommand) Execute(args []string) error {

stylesheet := styling.NewStylesheetFromConfig(configData.Stylesheet)

controller := NewController(initialDay, envData, categoryStyling, *stylesheet, stderrLogger, tuiLogger)

// now that the screen is initialized, we'll always want the TUI logger, so
// we're making it the global logger
previouslySetLogger := log.Logger
log.Logger = tuiLogger
log.Debug().Msg("set up logging to only TUI")

controller, err := NewController(initialDay, envData, categoryStyling, *stylesheet)
if err != nil {
log.Logger = previouslySetLogger
log.Error().Err(err).Msgf("something went wrong setting up the TUI, will check unpublished logs and return error")

// The TUI was perhaps not set up and we have to assume that the logs have not been written anywhere.
// To inform the user, we'll print the logs to stderr.
unpublishedLog := potatolog.GlobalMemoryLogReaderWriter.Get()
log.Warn().Msgf("have %d unpublished log entries which will be published now", len(unpublishedLog))
for _, entry := range unpublishedLog {
catchupLogger := log.Logger.With().Str("source", "catchup").Logger()

e := func() *zerolog.Event {
switch entry["level"] {
case "trace":
return catchupLogger.Trace()
case "debug":
return catchupLogger.Debug()
case "info":
return catchupLogger.Info()
case "warn":
return catchupLogger.Warn()
case "error":
return catchupLogger.Error()
}
return catchupLogger.Error()
}()

getEntryAsString := func(id string) string {
untyped, ok := entry[id]
if !ok {
return "<noentry>"
}
if str, ok := untyped.(string); ok {
return str
}
return fmt.Sprintf("<nonstring>: %v", untyped)
}
msg := getEntryAsString("message")
caller := getEntryAsString("caller")
timestamp := getEntryAsString("time")
e = e.Str("true-caller", caller).Str("true-timestamp", timestamp)
for k, v := range entry {
if k == "message" || k == "caller" || k == "timestamp" {
continue
}
e = e.Interface(k, v)
}
e.Msg(msg)
}
return fmt.Errorf("could not set up TUI (%w)", err)
}

controller.Run()
return nil
Expand Down

0 comments on commit b7aea34

Please sign in to comment.