Improve markdown stashing and spinner logic in file listing

ui/stash.go

@@ -115,7 +115,6 @@ type stashModel struct {
 	noteInput          textinput.Model
 	filterInput        textinput.Model
 	stashFullyLoaded   bool // have we loaded all available stashed documents from the server?
-	loadingFromNetwork bool // are we currently loading something from the network?
 	viewState          stashViewState
 	filterState        filterState
 	selectionState     selectionState
@@ -467,7 +466,6 @@ func newStashModel(general *general) stashModel {
 		filterInput: si,
 		serverPage:  1,
 		loaded:      NewDocTypeSet(),
-		loadingFromNetwork: true,
 		sections:    s,
 	}
 
@@ -487,7 +485,6 @@ func (m stashModel) update(msg tea.Msg) (stashModel, tea.Cmd) {
 		m.err = msg.err
 		m.loaded.Add(StashedDoc) // still done, albeit unsuccessfully
 		m.stashFullyLoaded = true
-		m.loadingFromNetwork = false
 
 	case newsLoadErrMsg:
 		m.err = msg.err
@@ -507,7 +504,6 @@ func (m stashModel) update(msg tea.Msg) (stashModel, tea.Cmd) {
 		switch msg := msg.(type) {
 		case gotStashMsg:
 			m.loaded.Add(StashedDoc)
-			m.loadingFromNetwork = false
 			docs = wrapMarkdowns(StashedDoc, msg)
 
 			if len(msg) == 0 {
@@ -551,13 +547,12 @@ func (m stashModel) update(msg tea.Msg) (stashModel, tea.Cmd) {
 		return m, nil
 
 	case spinner.TickMsg:
-		condition := !m.loadingDone() ||
-			m.loadingFromNetwork ||
-			m.viewState == stashStateLoadingDocument ||
-			len(m.general.filesStashed) > 0 ||
-			m.spinner.Visible()
+		loading := !m.loadingDone()
+		stashing := m.general.isStashing()
+		openingDocument := m.viewState == stashStateLoadingDocument
+		spinnerVisible := m.spinner.Visible()
 
-		if condition {
+		if loading || stashing || openingDocument || spinnerVisible {
 			newSpinnerModel, cmd := m.spinner.Update(msg)
 			m.spinner = newSpinnerModel
 			cmds = append(cmds, cmd)
@@ -572,15 +567,13 @@ func (m stashModel) update(msg tea.Msg) (stashModel, tea.Cmd) {
 			}
 		}
 
+	// Note: mechanical stuff related to stash success is handled in the parent
+	// update function.
 	case stashSuccessMsg:
-		md := markdown(msg)
-		m.addMarkdowns(&md)
-
-		if m.isFiltering() {
-			cmds = append(cmds, filterMarkdowns(m))
-		}
 		cmds = append(cmds, m.newStatusMessage("Stashed!"))
 
+	// Note: mechanical stuff related to stash failure is handled in the parent
+	// update function.
 	case stashFailMsg:
 		cmds = append(cmds, m.newStatusMessage("Couldn’t stash :("))
 
@@ -747,6 +740,7 @@ func (m *stashModel) handleDocumentBrowsing(msg tea.Msg) tea.Cmd {
 
 			// Checks passed; perform the stash
 			m.general.filesStashed[md.localID] = struct{}{}
+			m.general.filesStashing[md.localID] = struct{}{}
 			cmds = append(cmds, stashDocument(m.general.cc, *md))
 
 			if m.loadingDone() && !m.spinner.Visible() {
@@ -812,14 +806,6 @@ func (m *stashModel) handleDocumentBrowsing(msg tea.Msg) tea.Cmd {
 		m.setCursor(max(0, itemsOnPage-1))
 	}
 
-	// If we're on the last page and we haven't loaded everything, get
-	// more stuff.
-	if m.paginator().OnLastPage() && !m.loadingFromNetwork && !m.stashFullyLoaded {
-		m.serverPage++
-		m.loadingFromNetwork = true
-		cmds = append(cmds, loadStash(*m))
-	}
-
 	return tea.Batch(cmds...)
 }
 
@@ -973,7 +959,7 @@ func (m stashModel) view() string {
 	case stashStateReady:
 
 		loadingIndicator := " "
-		if !m.localOnly() && (!m.loadingDone() || m.loadingFromNetwork || m.spinner.Visible()) {
+		if !m.localOnly() && (!m.loadingDone() || m.spinner.Visible()) {
 			loadingIndicator = m.spinner.View()
 		}
 

ui/ui.go

@@ -169,6 +169,14 @@ type general struct {
 	// Local IDs of files stashed this session. We treat this like a set,
 	// ignoring the value portion with an empty struct.
 	filesStashed map[ksuid.KSUID]struct{}
+
+	// Files currently being stashed. We remove files from this set once
+	// a stash operation has either succeeded or failed.
+	filesStashing map[ksuid.KSUID]struct{}
+}
+
+func (g general) isStashing() bool {
+	return len(g.filesStashing) > 0
 }
 
 type model struct {
@@ -199,7 +207,7 @@ func (m *model) unloadDocument() []tea.Cmd {
 		batch = append(batch, tea.ClearScrollArea)
 	}
 
-	if !m.stash.loadingDone() || m.stash.loadingFromNetwork {
+	if !m.stash.loadingDone() {
 		batch = append(batch, spinner.Tick)
 	}
 	return batch
@@ -222,6 +230,7 @@ func newModel(cfg Config) tea.Model {
 		cfg:           cfg,
 		authStatus:    authConnecting,
 		filesStashed:  make(map[ksuid.KSUID]struct{}),
+		filesStashing: make(map[ksuid.KSUID]struct{}),
 	}
 
 	return model{
@@ -362,7 +371,6 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 
 			// Even though it failed, news/stash loading is finished
 			m.stash.loaded.Add(StashedDoc, NewsDoc)
-			m.stash.loadingFromNetwork = false
 		}
 
 	case keygenFailedMsg:
@@ -377,7 +385,6 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 
 		// Even though it failed, news/stash loading is finished
 		m.stash.loaded.Add(StashedDoc, NewsDoc)
-		m.stash.loadingFromNetwork = false
 
 	case keygenSuccessMsg:
 		// The keygen's done, so let's try initializing the charm client again
@@ -429,22 +436,20 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		cmds = append(cmds, findNextLocalFile(m))
 
 	case stashSuccessMsg:
-		// Something was stashed outside the file listing view. Update the
-		// stash listing but don't run an actual update on the stash since we
-		// don't want to trigger the status message and generally don't want
-		// any other effects.
-		if m.state == stateShowDocument {
+		// Common handling that should happen regardless of application state
 		md := markdown(msg)
 		m.stash.addMarkdowns(&md)
 		m.general.filesStashed[msg.localID] = struct{}{}
+		delete(m.general.filesStashing, md.localID)
 
 		if m.stash.isFiltering() {
 			cmds = append(cmds, filterMarkdowns(m.stash))
 		}
-		}
 
 	case stashFailMsg:
+		// Common handling that should happen regardless of application state
 		delete(m.general.filesStashed, msg.markdown.localID)
+		delete(m.general.filesStashing, msg.markdown.localID)
 
 	case filteredMarkdownMsg:
 		if m.state == stateShowDocument {