From a2dc756d682e0efd9c800035cc1376aa18295eec Mon Sep 17 00:00:00 2001 From: ekideno Date: Sat, 16 May 2026 19:39:35 +0300 Subject: [PATCH] chore: Add better error handling using ErrorPopup Refactored error handling across the codebase to use the existing ErrorPopup component via utils.ReturnError() function. This provides a consistent user experience when errors occur in the TUI. Changes: - Updated internal/models files to return errors from Update() methods using utils.ReturnError() which displays errors via ErrorPopup - Modified file operation handlers (create, delete, rename, move) to return errors instead of calling os.Exit(1) - Updated daily task operations (GetItems, WriteItems, handlers) to propagate errors properly - Fixed viewer ReadFile() to return errors instead of printing them - Updated constructors (NewDaily, NewHome, NewFileExplorer) to return errors for proper initialization error handling - Improved CLI commands to return errors using standard Go error handling All error handling now follows the pattern specified in issue #63: utils.ReturnError() is only called from Update() methods, and errors are properly propagated through the call chain. Fixes #63 --- cmd/tasks/create.go | 10 ++++- cmd/tasks/delete.go | 9 +++- cmd/tasks/edit.go | 9 +++- cmd/tasks/list.go | 10 ++++- internal/models/daily/daily.go | 36 +++++++++++----- internal/models/daily/handlers.go | 24 +++++------ internal/models/daily/items.go | 41 ++++++++++++------- internal/models/fileExplorer/fileExplorer.go | 9 ++-- internal/models/filePopup/handlers.go | 43 +++++++++++--------- internal/models/homeModel/homeModel.go | 11 +++-- internal/models/rootModel.go | 15 ++++++- internal/models/viewer/viewer.go | 21 ++++++---- 12 files changed, 157 insertions(+), 81 deletions(-) diff --git a/cmd/tasks/create.go b/cmd/tasks/create.go index d6f08f5..89c9019 100644 --- a/cmd/tasks/create.go +++ b/cmd/tasks/create.go @@ -63,7 +63,11 @@ func CreatCmd() *cobra.Command { form := huh.NewForm(grps...).WithTheme(styles.HuhTheme()) form.Run() - tasks := daily.GetItems() + tasks, err := daily.GetItems() + if err != nil { + return fmt.Errorf("failed to get tasks: %w", err) + } + tasks = append(tasks, daily.Task{ TaskTitle: opts.Title, TaskType: opts.Type, @@ -71,7 +75,9 @@ func CreatCmd() *cobra.Command { Status: opts.Status, }) - daily.WriteItems(tasks) + if err := daily.WriteItems(tasks); err != nil { + return fmt.Errorf("failed to write tasks: %w", err) + } return nil }, diff --git a/cmd/tasks/delete.go b/cmd/tasks/delete.go index a4f9850..fbf93a9 100644 --- a/cmd/tasks/delete.go +++ b/cmd/tasks/delete.go @@ -26,7 +26,10 @@ func DeleteCmd() *cobra.Command { return fmt.Errorf("failed to load config: %v\n\n%s", err, "Try running the `toney init` command and try again.") } - tasks := daily.GetItems() + tasks, err := daily.GetItems() + if err != nil { + return fmt.Errorf("failed to get tasks: %w", err) + } options := make([]huh.Option[int], 0) for _, v := range tasks { @@ -58,7 +61,9 @@ func DeleteCmd() *cobra.Command { fmt.Printf("Deleted Task: `%s`\n", tasks[opts.ID].Title()) tasks = slices.Delete(tasks, opts.ID-1, opts.ID) - daily.WriteItems(tasks) + if err := daily.WriteItems(tasks); err != nil { + return fmt.Errorf("failed to write tasks: %w", err) + } return nil }, diff --git a/cmd/tasks/edit.go b/cmd/tasks/edit.go index 92d3744..dd3b56e 100644 --- a/cmd/tasks/edit.go +++ b/cmd/tasks/edit.go @@ -29,7 +29,10 @@ func EditCmd() *cobra.Command { return fmt.Errorf("failed to load config: %v\n\n%s", err, "Try running the `toney init` command and try again.") } - tasks := daily.GetItems() + tasks, err := daily.GetItems() + if err != nil { + return fmt.Errorf("failed to get tasks: %w", err) + } options := make([]huh.Option[int], 0) for k, v := range tasks { @@ -113,7 +116,9 @@ func EditCmd() *cobra.Command { tasks[opts.ID] = task - daily.WriteItems(tasks) + if err := daily.WriteItems(tasks); err != nil { + return fmt.Errorf("failed to write tasks: %w", err) + } return nil }, diff --git a/cmd/tasks/list.go b/cmd/tasks/list.go index 6da78c6..17edd1b 100644 --- a/cmd/tasks/list.go +++ b/cmd/tasks/list.go @@ -34,9 +34,15 @@ func ListCmd() *cobra.Command { dl := CmdDelegate{Verbose: opts.Verbose} ht := dl.Height() + dl.Spacing() - tasks := daily.TaskToItems(daily.GetItems()) - lst := list.New(tasks, dl, 1000, len(tasks)*ht) + tasks, err := daily.GetItems() + if err != nil { + return fmt.Errorf("failed to get tasks: %w", err) + } + + taskItems := daily.TaskToItems(tasks) + + lst := list.New(taskItems, dl, 1000, len(taskItems)*ht) lst.SetShowTitle(false) lst.SetShowHelp(false) lst.SetShowFilter(false) diff --git a/internal/models/daily/daily.go b/internal/models/daily/daily.go index c0533ad..aeb64b0 100644 --- a/internal/models/daily/daily.go +++ b/internal/models/daily/daily.go @@ -7,6 +7,7 @@ import ( "github.com/SourcewareLab/Toney/v2/internal/messages" taskpopup "github.com/SourcewareLab/Toney/v2/internal/models/taskPopup" "github.com/SourcewareLab/Toney/v2/internal/styles" + "github.com/SourcewareLab/Toney/v2/internal/utils" "github.com/charmbracelet/bubbles/help" "github.com/charmbracelet/bubbles/key" "github.com/charmbracelet/bubbles/list" @@ -25,8 +26,11 @@ type Daily struct { Help help.Model } -func NewDaily(w int, h int) *Daily { - tasks := GetItems() +func NewDaily(w int, h int) (*Daily, error) { + tasks, err := GetItems() + if err != nil { + return nil, err + } lst := list.New(TaskToItems(tasks), TaskDelegate{}, w/2, 2*h/3) km := list.DefaultKeyMap() @@ -42,7 +46,7 @@ func NewDaily(w int, h int) *Daily { Tasks: tasks, Keymap: keymap.NewDailyTaskMap(), Help: help.New(), - } + }, nil } func (m *Daily) Init() tea.Cmd { @@ -52,20 +56,27 @@ func (m *Daily) Init() tea.Cmd { func (m *Daily) Update(msg tea.Msg) (tea.Model, tea.Cmd) { switch msg := msg.(type) { case messages.TaskPopupMessage: + var err error switch msg.Type { case enums.CreateRecurring: fallthrough case enums.CreateUnique: - m.CreateTask(msg, msg.Type == enums.CreateUnique) + err = m.CreateTask(msg, msg.Type == enums.CreateUnique) case enums.Delete: - m.DeleteTask(msg) + err = m.DeleteTask(msg) case enums.ChangeStatus: - m.StatusChangeTask(msg) + err = m.StatusChangeTask(msg) case enums.Edit: - m.EditTask(msg) + err = m.EditTask(msg) + } + + if err != nil { + return m, utils.ReturnError("Daily", "Task Operation Error", err) } - m.Refresh() + if err := m.Refresh(); err != nil { + return m, utils.ReturnError("Daily", "Error Refreshing Tasks", err) + } m.ShowPopup = false return m, nil case tea.KeyMsg: @@ -144,8 +155,12 @@ func (m *Daily) View() string { return lipgloss.JoinVertical(lipgloss.Left, main, help) } -func (m *Daily) Refresh() { - m.Tasks = GetItems() +func (m *Daily) Refresh() error { + tasks, err := GetItems() + if err != nil { + return err + } + m.Tasks = tasks lst := list.New(TaskToItems(m.Tasks), TaskDelegate{}, m.Width/2, 2*m.Height/3) km := list.DefaultKeyMap() @@ -155,4 +170,5 @@ func (m *Daily) Refresh() { lst.SetShowTitle(false) m.List = lst + return nil } diff --git a/internal/models/daily/handlers.go b/internal/models/daily/handlers.go index 416af8f..2e198dc 100644 --- a/internal/models/daily/handlers.go +++ b/internal/models/daily/handlers.go @@ -7,7 +7,7 @@ import ( "github.com/SourcewareLab/Toney/v2/internal/messages" ) -func (m Daily) CreateTask(msg messages.TaskPopupMessage, isUnique bool) { +func (m Daily) CreateTask(msg messages.TaskPopupMessage, isUnique bool) error { task := Task{ TaskTitle: msg.Title, TaskDesc: msg.Desc, @@ -22,42 +22,42 @@ func (m Daily) CreateTask(msg messages.TaskPopupMessage, isUnique bool) { m.Tasks = append(m.Tasks, task) - WriteItems(m.Tasks) + return WriteItems(m.Tasks) } -func (m Daily) DeleteTask(msg messages.TaskPopupMessage) { +func (m Daily) DeleteTask(msg messages.TaskPopupMessage) error { if !msg.IsDeleted { - return + return nil } item := m.List.SelectedItem() task, ok := item.(Task) if !ok { // Making sure that item is of type Task - return + return nil } m.Tasks = slices.Delete(m.Tasks, task.ID-1, task.ID) - WriteItems(m.Tasks) + return WriteItems(m.Tasks) } -func (m Daily) StatusChangeTask(msg messages.TaskPopupMessage) { +func (m Daily) StatusChangeTask(msg messages.TaskPopupMessage) error { item := m.List.SelectedItem() task, ok := item.(Task) if !ok { // Making sure that item is of type Task - return + return nil } task.Status = msg.Status m.Tasks[task.ID-1] = task - WriteItems(m.Tasks) + return WriteItems(m.Tasks) } -func (m Daily) EditTask(msg messages.TaskPopupMessage) { +func (m Daily) EditTask(msg messages.TaskPopupMessage) error { task := Task{ TaskTitle: msg.Title, TaskDesc: msg.Desc, @@ -68,12 +68,12 @@ func (m Daily) EditTask(msg messages.TaskPopupMessage) { oldTask, ok := item.(Task) if !ok { // Making sure that item is of type Task - return + return nil } task.Status = oldTask.Status m.Tasks[oldTask.ID-1] = task - WriteItems(m.Tasks) + return WriteItems(m.Tasks) } diff --git a/internal/models/daily/items.go b/internal/models/daily/items.go index 3c8a5f1..ea05aee 100644 --- a/internal/models/daily/items.go +++ b/internal/models/daily/items.go @@ -1,7 +1,6 @@ package daily import ( - "fmt" "os" "path/filepath" "slices" @@ -36,60 +35,74 @@ func TaskToItems(tasks []Task) []list.Item { return list } -func GetItems() []Task { +func GetItems() ([]Task, error) { path := GetPath() _, err := os.Stat(path) if os.IsNotExist(err) { f, err2 := os.Create(path) if err2 != nil { - fmt.Println(err2.Error()) + return nil, err2 } prevPath := getMostRecentDailyPath() if prevPath != "" { content, err2 := os.ReadFile(prevPath) if err2 != nil { - fmt.Println(err2.Error()) + return nil, err2 } var allTasks []Task - csvutil.Unmarshal(content, &allTasks) + if err := csvutil.Unmarshal(content, &allTasks); err != nil { + return nil, err + } tasks := filterRolloverTasks(allTasks) data, err2 := csvutil.Marshal(tasks) if err2 != nil { - fmt.Println(err2.Error()) + return nil, err2 } - f.Write(data) + if _, err := f.Write(data); err != nil { + return nil, err + } } } else if err != nil { - fmt.Println("Error: ", err.Error()) + return nil, err } - content, _ := os.ReadFile(path) + content, err := os.ReadFile(path) + if err != nil { + return nil, err + } tasks := make([]Task, 0) err = csvutil.Unmarshal(content, &tasks) if err != nil { - fmt.Println(err.Error()) + return nil, err } slices.SortStableFunc(tasks, func(a, b Task) int { return getStatusOrder(a.Status) - getStatusOrder(b.Status) }) - return tasks + return tasks, nil } -func WriteItems(tasks []Task) { +func WriteItems(tasks []Task) error { path := GetPath() - data, _ := csvutil.Marshal(tasks) + data, err := csvutil.Marshal(tasks) + if err != nil { + return err + } + + if err := os.WriteFile(path, data, 0o644); err != nil { + return err + } - os.WriteFile(path, data, 0o644) + return nil } func GetPath() string { diff --git a/internal/models/fileExplorer/fileExplorer.go b/internal/models/fileExplorer/fileExplorer.go index 70e7ad4..0a1531a 100644 --- a/internal/models/fileExplorer/fileExplorer.go +++ b/internal/models/fileExplorer/fileExplorer.go @@ -1,8 +1,6 @@ package fileexplorer import ( - "fmt" - "os" "os/exec" "path/filepath" "strings" @@ -36,11 +34,10 @@ type FileExplorer struct { Keymap keymap.ExplorerKeyMap } -func NewFileExplorer(w int, h int) *FileExplorer { +func NewFileExplorer(w int, h int) (*FileExplorer, error) { root, err := filetree.CreateTree() if err != nil { - fmt.Println(err.Error()) - os.Exit(1) + return nil, err } return &FileExplorer{ @@ -52,7 +49,7 @@ func NewFileExplorer(w int, h int) *FileExplorer { CurrentIndex: 0, VisibleNodes: filetree.FlattenVisibleTree(root), Keymap: keymap.NewExplorerKeyMap(), - } + }, nil } func (m FileExplorer) Init() tea.Cmd { diff --git a/internal/models/filePopup/handlers.go b/internal/models/filePopup/handlers.go index a975409..ae166b2 100644 --- a/internal/models/filePopup/handlers.go +++ b/internal/models/filePopup/handlers.go @@ -8,26 +8,32 @@ import ( "github.com/SourcewareLab/Toney/v2/internal/enums" filetree "github.com/SourcewareLab/Toney/v2/internal/fileTree" "github.com/SourcewareLab/Toney/v2/internal/messages" + "github.com/SourcewareLab/Toney/v2/internal/utils" tea "github.com/charmbracelet/bubbletea" ) func HandleEnter(m *FilePopup) (tea.Model, tea.Cmd) { path := GetPath(m.Node) + var err error switch m.Type { case enums.FileCreate: - Create(path, m.TextInput.Value(), m.Node) + err = Create(path, m.TextInput.Value(), m.Node) case enums.FileDelete: - Delete(path[0:len(path)-1], m.TextInput.Value()) + err = Delete(path[0:len(path)-1], m.TextInput.Value()) case enums.FileRename: - Rename(path[0:len(path)-1], m.TextInput.Value()) + err = Rename(path[0:len(path)-1], m.TextInput.Value()) case enums.FileMove: - Move(path[0:len(path)-1], m.TextInput.Value()) + err = Move(path[0:len(path)-1], m.TextInput.Value()) default: fmt.Println("default") } + if err != nil { + return m, utils.ReturnError("FilePopup", "File Operation Error", err) + } + return m, tea.Batch(func() tea.Msg { return messages.HidePopupMessage{} }, @@ -36,37 +42,37 @@ func HandleEnter(m *FilePopup) (tea.Model, tea.Cmd) { }) } -func Move(path string, value string) { +func Move(path string, value string) error { err := os.Rename(path, value) if err != nil { - fmt.Println(err.Error()) - os.Exit(1) + return err } + return nil } -func Rename(path string, value string) { +func Rename(path string, value string) error { newpathArr := strings.Split(path, "/") newpath := strings.Join(newpathArr[0:len(newpathArr)-1], "/") + "/" + value err := os.Rename(path, newpath) if err != nil { - fmt.Println(err.Error()) - os.Exit(1) + return err } + return nil } -func Delete(path string, value string) { +func Delete(path string, value string) error { if value != "y" { - return + return nil } err := os.Remove(path) if err != nil { - fmt.Println(err.Error()) - os.Exit(1) + return err } + return nil } -func Create(path string, value string, n *filetree.Node) { +func Create(path string, value string, n *filetree.Node) error { if !n.IsDirectory { pathArr := strings.Split(path, "/") pathArr = pathArr[0 : len(pathArr)-2] @@ -76,16 +82,15 @@ func Create(path string, value string, n *filetree.Node) { if strings.HasSuffix(value, "/") { err := os.MkdirAll(path+value, 0o755) if err != nil { - fmt.Println(err.Error()) - // os.Exit(1) + return err } } else { _, err := os.Create(path + value) if err != nil { - fmt.Println(err.Error()) - os.Exit(1) + return err } } + return nil } func MapExpanded(new *filetree.Node, old *filetree.Node) { diff --git a/internal/models/homeModel/homeModel.go b/internal/models/homeModel/homeModel.go index bd3c762..f73d167 100644 --- a/internal/models/homeModel/homeModel.go +++ b/internal/models/homeModel/homeModel.go @@ -31,22 +31,27 @@ type HomeModel struct { Help help.Model } -func NewHome(w int, h int) *HomeModel { +func NewHome(w int, h int) (*HomeModel, error) { home, _ := os.UserHomeDir() path := filepath.Join(home, config.AppConfig.General.NotesDir) files, _ := filetree.ListFilesRel(path) + fileExplorer, err := fileexplorer.NewFileExplorer(w, h) + if err != nil { + return nil, err + } + return &HomeModel{ Width: w, Height: h, ShowFinder: false, FocusOn: enums.File, - FileExplorer: fileexplorer.NewFileExplorer(w, h), + FileExplorer: fileExplorer, Viewer: viewer.NewViewer(w, h), Finder: fzf.NewFzf(files, w, h), Keymap: keymap.NewHomeKeyMap(), Help: help.New(), - } + }, nil } func (m HomeModel) Init() tea.Cmd { diff --git a/internal/models/rootModel.go b/internal/models/rootModel.go index bc7337a..9faf1e7 100644 --- a/internal/models/rootModel.go +++ b/internal/models/rootModel.go @@ -180,9 +180,20 @@ func (m *RootModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.Diary.Update(msg) m.Daily.Update(msg) } else { - m.Home = homemodel.NewHome(msg.Width, msg.Height) + homeModel, err := homemodel.NewHome(msg.Width, msg.Height) + if err != nil { + return m, utils.ReturnError("Root", "Error Initializing Home", err) + } + m.Home = homeModel + m.Menu = menu.NewMenu(msg.Width, msg.Height) - m.Daily = daily.NewDaily(msg.Width, msg.Height) + + dailyModel, err := daily.NewDaily(msg.Width, msg.Height) + if err != nil { + return m, utils.ReturnError("Root", "Error Initializing Daily Tasks", err) + } + m.Daily = dailyModel + m.Diary = diary.NewDiary(msg.Width, msg.Height) } diff --git a/internal/models/viewer/viewer.go b/internal/models/viewer/viewer.go index fd6eab9..319d882 100644 --- a/internal/models/viewer/viewer.go +++ b/internal/models/viewer/viewer.go @@ -1,7 +1,6 @@ package viewer import ( - "fmt" "os" "strings" @@ -9,6 +8,7 @@ import ( "github.com/SourcewareLab/Toney/v2/internal/keymap" "github.com/SourcewareLab/Toney/v2/internal/messages" "github.com/SourcewareLab/Toney/v2/internal/models/fzf" + "github.com/SourcewareLab/Toney/v2/internal/utils" "github.com/SourcewareLab/Toney/v2/internal/colors" "github.com/charmbracelet/bubbles/key" @@ -68,12 +68,20 @@ func (m Viewer) Init() tea.Cmd { func (m *Viewer) Update(msg tea.Msg) (tea.Model, tea.Cmd) { switch msg := msg.(type) { case messages.EditorClose: - m.Content = m.ReadFile() + content, err := m.ReadFile() + if err != nil { + return m, utils.ReturnError("Viewer", "Error Reading File", err) + } + m.Content = content m.Viewport.SetContent(m.RenderMarkdown(m.Content, m.Width)) return m, nil case messages.ChangeFileMessage: m.Path = msg.Path - m.Content = m.ReadFile() + content, err := m.ReadFile() + if err != nil { + return m, utils.ReturnError("Viewer", "Error Reading File", err) + } + m.Content = content m.Viewport.SetContent(m.RenderMarkdown(m.Content, m.Width)) m.Viewport.YOffset = 0 m.Highlighted = -1 @@ -146,16 +154,15 @@ func (m *Viewer) Header() string { return "" } -func (m *Viewer) ReadFile() []string { // Change to editor type when config done +func (m *Viewer) ReadFile() ([]string, error) { path := strings.TrimSuffix(m.Path, "/") content, err := os.ReadFile(path) if err != nil { - fmt.Println(err.Error()) - content = ([]byte)(fmt.Sprintf("An error occured while reading the file:%s\n%s", m.Path, err.Error())) + return nil, err } - return strings.Split(string(content), "\n") + return strings.Split(string(content), "\n"), nil } func (m *Viewer) RenderMarkdown(md []string, width int) string {