🚧 Fixed some bugs, major ones remain

CHANGELOG.md

@@ -9,12 +9,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Link and heading lines are wrapped just like regular text lines
 - Wrapped list items are indented to stay behind the bullet (#35)
 - Certificate expiry date is stored when the cert IDs match (#39)
+- What link was selected is remembered as you browse through history
 
 ### Changed
 - Pages are rewrapped dynamically, whenever the terminal size changes (#33)
 
 ### Fixed
-- Many potential loading race conditions eliminated
+- Many potential network and display race conditions eliminated
+- Whether a tab is loading stays indicated when you switch away from it and go back
 
 ## [1.2.0] - 2020-07-02
 ### Added

NOTES.md

@@ -1,6 +1,12 @@
 # Notes
 
 - URL for each tab should not be stored as a string - in the current code there's lots of reparsing the URL
+- Switch to UUIDs for each tab maybe? So that if `handleURL` completes after the tab is closed (and reopened) it doesn't go anywhere
+  - New UUID every time a new page is loaded?
+
+## Bugs
+- Can't go back or do other things while page is loading - need a way to stop `handleURL`
+- `handleURL` will reference a tab that doesn't exist and cause a panic if the tab is closed
 
 ## Upstream Bugs
 - Wrapping messes up on brackets

cache/cache.go

@@ -111,7 +111,7 @@ func NumPages() int {
 }
 
 // Get returns the page struct, and a bool indicating if the page was in the cache or not.
-// An empty page struct is returned if the page isn't in the cache
+// An empty page struct is returned if the page isn't in the cache.
 func Get(url string) (*structs.Page, bool) {
 	lock.RLock()
 	defer lock.RUnlock()

display/bookmarks.go

@@ -96,7 +96,7 @@ func openBkmkModal(name string, exists bool) (string, int) {
 }
 
 // Bookmarks displays the bookmarks page on the current tab.
-func Bookmarks() {
+func Bookmarks(tab int) {
 	// Gather bookmarks
 	rawContent := "# Bookmarks\r\n\r\n"
 	m, keys := bookmarks.All()
@@ -114,6 +114,7 @@ func Bookmarks() {
 		Mediatype: structs.TextGemini,
 	}
 	setPage(curTab, &page)
+	tabs[tab].applyBottomBar()
 }
 
 // addBookmark goes through the process of adding a bookmark for the current page.

display/display.go

@@ -224,88 +224,100 @@ func Init() {
 			return event
 		}
 
-		// History arrow keys
-		if event.Modifiers() == tcell.ModAlt {
-			if event.Key() == tcell.KeyLeft {
-				histBack()
-				return nil
-			}
-			if event.Key() == tcell.KeyRight {
-				histForward()
-				return nil
-			}
-		}
+		if tabs[curTab].mode == tabModeDone {
+			// All the keys and operations that can only work while NOT loading
 
-		switch event.Key() {
-		case tcell.KeyCtrlT:
-			if tabs[curTab].mode == modeLinkSelect {
-				next, err := resolveRelLink(curTab, tabs[curTab].page.Url, tabs[curTab].selected)
-				if err != nil {
-					Error("URL Error", err.Error())
+			// History arrow keys
+			if event.Modifiers() == tcell.ModAlt {
+				if event.Key() == tcell.KeyLeft {
+					histBack()
+					return nil
+				}
+				if event.Key() == tcell.KeyRight {
+					histForward()
 					return nil
 				}
-				NewTab()
-				URL(next)
-			} else {
-				NewTab()
 			}
-			return nil
+
+			switch event.Key() {
+			case tcell.KeyCtrlT:
+				if tabs[curTab].page.Mode == structs.ModeLinkSelect {
+					next, err := resolveRelLink(curTab, tabs[curTab].page.Url, tabs[curTab].page.Selected)
+					if err != nil {
+						Error("URL Error", err.Error())
+						return nil
+					}
+					NewTab()
+					URL(next)
+				} else {
+					NewTab()
+				}
+				return nil
+			case tcell.KeyCtrlR:
+				Reload()
+				return nil
+			case tcell.KeyCtrlH:
+				URL(viper.GetString("a-general.home"))
+				return nil
+			case tcell.KeyCtrlB:
+				Bookmarks(curTab)
+				tabs[curTab].addToHistory("about:bookmarks")
+				return nil
+			case tcell.KeyCtrlD:
+				go addBookmark()
+				return nil
+			case tcell.KeyPgUp:
+				tabs[curTab].pageUp()
+				return nil
+			case tcell.KeyPgDn:
+				tabs[curTab].pageDown()
+				return nil
+			case tcell.KeyRune:
+				// Regular key was sent
+				switch string(event.Rune()) {
+				case " ":
+					// Space starts typing, like Bombadillo
+					bottomBar.SetLabel("[::b]URL/Num./Search: [::-]")
+					bottomBar.SetText("")
+					// Don't save bottom bar, so that whenever you switch tabs, it's not in that mode
+					App.SetFocus(bottomBar)
+					return nil
+				case "R":
+					Reload()
+					return nil
+				case "b":
+					histBack()
+					return nil
+				case "f":
+					histForward()
+					return nil
+				case "u":
+					tabs[curTab].pageUp()
+					return nil
+				case "d":
+					tabs[curTab].pageDown()
+					return nil
+				}
+			}
+		}
+		// All the keys and operations that can work while a tab IS loading
+
+		switch event.Key() {
 		case tcell.KeyCtrlW:
 			CloseTab()
 			return nil
-		case tcell.KeyCtrlR:
-			Reload()
-			return nil
-		case tcell.KeyCtrlH:
-			URL(viper.GetString("a-general.home"))
-			return nil
 		case tcell.KeyCtrlQ:
 			Stop()
 			return nil
-		case tcell.KeyCtrlB:
-			Bookmarks()
-			tabs[curTab].addToHistory("about:bookmarks")
-			return nil
-		case tcell.KeyCtrlD:
-			go addBookmark()
-			return nil
-		case tcell.KeyPgUp:
-			tabs[curTab].pageUp()
-			return nil
-		case tcell.KeyPgDn:
-			tabs[curTab].pageDown()
-			return nil
 		case tcell.KeyRune:
 			// Regular key was sent
 			switch string(event.Rune()) {
-			case " ":
-				// Space starts typing, like Bombadillo
-				bottomBar.SetLabel("[::b]URL/Num./Search: [::-]")
-				bottomBar.SetText("")
-				// Don't save bottom bar, so that whenever you switch tabs, it's not in that mode
-				App.SetFocus(bottomBar)
-				return nil
 			case "q":
 				Stop()
 				return nil
-			case "R":
-				Reload()
-				return nil
-			case "b":
-				histBack()
-				return nil
-			case "f":
-				histForward()
-				return nil
 			case "?":
 				Help()
 				return nil
-			case "u":
-				tabs[curTab].pageUp()
-				return nil
-			case "d":
-				tabs[curTab].pageDown()
-				return nil
 
 			// Shift+NUMBER keys, for switching to a specific tab
 			case "!":
@@ -340,6 +352,8 @@ func Init() {
 				return nil
 			}
 		}
+
+		// Let another element handle the event, it's not a special global key
 		return event
 	})
 }
@@ -367,10 +381,9 @@ func NewTab() {
 	}
 
 	curTab = NumTabs()
-	reformatPage(newTabPage)
 
-	tabs[curTab] = makeNewTab()
-	tabs[curTab].page = newTabPage
+	tabs = append(tabs, makeNewTab())
+	setPage(curTab, newTabPage)
 
 	// Can't go backwards, but this isn't the first page either.
 	// The first page will be the next one the user goes to.
@@ -436,9 +449,12 @@ func CloseTab() {
 	}
 	tabRow.Highlight(strconv.Itoa(curTab)).ScrollToHighlight()
 
-	// Set previous tab's bottomBar state
+	// Restore previous tab's state
+	tabs[curTab].applySelected()
 	tabs[curTab].applyBottomBar()
 
+	App.SetFocus(tabs[curTab].view)
+
 	// Just in case
 	App.Draw()
 }
@@ -466,8 +482,11 @@ func SwitchTab(tab int) {
 	reformatPageAndSetView(curTab, tabs[curTab].page)
 	tabPages.SwitchToPage(strconv.Itoa(curTab))
 	tabRow.Highlight(strconv.Itoa(curTab)).ScrollToHighlight()
+	tabs[curTab].applySelected()
 	tabs[curTab].applyBottomBar()
 
+	App.SetFocus(tabs[curTab].view)
+
 	// Just in case
 	App.Draw()
 }
@@ -493,7 +512,7 @@ func URL(u string) {
 	// Some code is copied in followLink()
 
 	if u == "about:bookmarks" {
-		Bookmarks()
+		Bookmarks(curTab)
 		tabs[curTab].addToHistory("about:bookmarks")
 		return
 	}

display/history.go

@@ -9,6 +9,7 @@ func histForward() {
 	go func(tab int) {
 		handleURL(tab, tabs[tab].history.urls[tabs[tab].history.pos]) // Load that position in history
 		tabs[tab].applyScroll()
+		tabs[tab].applySelected()
 		if tab == curTab {
 			// Display the bottomBar state that handleURL set
 			tabs[tab].applyBottomBar()
@@ -25,6 +26,7 @@ func histBack() {
 	go func(tab int) {
 		handleURL(tab, tabs[tab].history.urls[tabs[tab].history.pos]) // Load that position in history
 		tabs[tab].applyScroll()
+		tabs[tab].applySelected()
 		if tab == curTab {
 			// Display the bottomBar state that handleURL set
 			tabs[tab].applyBottomBar()

display/private.go

@@ -71,7 +71,7 @@ func followLink(tab int, prev, next string) {
 
 	// Copied from URL()
 	if next == "about:bookmarks" {
-		Bookmarks()
+		Bookmarks(tab)
 		tabs[tab].addToHistory("about:bookmarks")
 		return
 	}
@@ -190,7 +190,7 @@ func handleURL(tab int, u string) (string, bool) {
 
 	// To allow linking to the bookmarks page, and history browsing
 	if u == "about:bookmarks" {
-		Bookmarks()
+		Bookmarks(tab)
 		return "about:bookmarks", true
 	}
 
@@ -241,9 +241,14 @@ func handleURL(tab int, u string) (string, bool) {
 	// Otherwise download it
 	bottomBar.SetText("Loading...")
 	tabs[tab].barText = "Loading..." // Save it too, in case the tab switches during loading
+	tabs[tab].mode = tabModeLoading
+	defer func(t int) {
+		tabs[t].mode = tabModeDone
+	}(tab)
 	App.Draw()
 
 	res, err := client.Fetch(u)
+
 	if err == client.ErrTofu {
 		if Tofu(parsed.Host) {
 			// They want to continue anyway
@@ -270,7 +275,7 @@ func handleURL(tab int, u string) (string, bool) {
 			tabs[tab].barText = tabs[tab].page.Url
 			return "", false
 		}
-		cache.Add(page)
+		go cache.Add(page)
 		setPage(tab, page)
 		return u, true
 	}

display/tab.go

@@ -13,11 +13,10 @@ import (
 type tabMode int
 
 const (
-	modeOff        tabMode = iota // Regular mode
-	modeLinkSelect                // When the enter key is pressed, allow for tab-based link navigation
+	tabModeDone tabMode = iota
+	tabModeLoading
 )
 
-// tabHist holds the history for a tab.
 type tabHistory struct {
 	urls []string
 	pos  int // Position: where in the list of URLs we are
@@ -27,11 +26,9 @@ type tabHistory struct {
 type tab struct {
 	page        *structs.Page
 	view        *cview.TextView
-	mode        tabMode
 	history     *tabHistory
+	mode        tabMode
 	reformatMut *sync.Mutex // Mutex for reformatting, so there's only one reformat job at once
-	selected    string      // The current text or link selected
-	selectedID  string      // The cview region ID for the selected text/link
 	barLabel    string      // The bottomBar label for the tab
 	barText     string      // The bottomBar text for the tab
 }
@@ -39,7 +36,7 @@ type tab struct {
 // makeNewTab initializes an tab struct with no content.
 func makeNewTab() *tab {
 	t := tab{
-		page: &structs.Page{},
+		page: &structs.Page{Mode: structs.ModeOff},
 		view: cview.NewTextView().
 			SetDynamicColors(true).
 			SetRegions(true).
@@ -48,9 +45,9 @@ func makeNewTab() *tab {
 			SetChangedFunc(func() {
 				App.Draw()
 			}),
-		mode:        modeOff,
 		history:     &tabHistory{},
 		reformatMut: &sync.Mutex{},
+		mode:        tabModeDone,
 	}
 	t.view.SetDoneFunc(func(key tcell.Key) {
 		// Altered from: https://gitlab.com/tslocum/cview/-/blob/master/demos/textview/main.go
@@ -58,34 +55,43 @@ func makeNewTab() *tab {
 
 		tab := curTab // Don't let it change in the middle of the code
 
-		defer tabs[tab].saveBottomBar()
-
-		if key == tcell.KeyEsc {
+		if key == tcell.KeyEsc && tabs[tab].mode == tabModeDone {
 			// Stop highlighting
-			tabs[tab].view.Highlight("")
 			bottomBar.SetLabel("")
 			bottomBar.SetText(tabs[tab].page.Url)
-			tabs[tab].mode = modeOff
+			tabs[tab].clearSelected()
+			tabs[tab].saveBottomBar()
+			return
+		}
+
+		if len(tabs[tab].page.Links) <= 0 {
+			// No links on page
+			return
 		}
 
 		currentSelection := tabs[tab].view.GetHighlights()
 		numSelections := len(tabs[tab].page.Links)
 
 		if key == tcell.KeyEnter {
-			if len(currentSelection) > 0 && len(tabs[tab].page.Links) > 0 {
+			if len(currentSelection) > 0 {
 				// A link was selected, "click" it and load the page it's for
 				bottomBar.SetLabel("")
-				tabs[tab].mode = modeOff
 				linkN, _ := strconv.Atoi(currentSelection[0])
+				tabs[tab].page.Selected = tabs[tab].page.Links[linkN]
+				tabs[tab].page.SelectedID = currentSelection[0]
 				followLink(tab, tabs[tab].page.Url, tabs[tab].page.Links[linkN])
 				return
 			} else {
+				// They've started link highlighting
+				tabs[tab].page.Mode = structs.ModeLinkSelect
+
 				tabs[tab].view.Highlight("0").ScrollToHighlight()
 				// Display link URL in bottomBar
 				bottomBar.SetLabel("[::b]Link: [::-]")
 				bottomBar.SetText(tabs[tab].page.Links[0])
-				tabs[tab].selected = tabs[tab].page.Links[0]
-				tabs[tab].selectedID = "0"
+				tabs[tab].saveBottomBar()
+				tabs[tab].page.Selected = tabs[tab].page.Links[0]
+				tabs[tab].page.SelectedID = "0"
 			}
 		} else if len(currentSelection) > 0 {
 			// There's still a selection, but a different key was pressed, not Enter
@@ -102,8 +108,9 @@ func makeNewTab() *tab {
 			// Display link URL in bottomBar
 			bottomBar.SetLabel("[::b]Link: [::-]")
 			bottomBar.SetText(tabs[tab].page.Links[index])
-			tabs[tab].selected = tabs[tab].page.Links[index]
-			tabs[tab].selectedID = currentSelection[0]
+			tabs[tab].saveBottomBar()
+			tabs[tab].page.Selected = tabs[tab].page.Links[index]
+			tabs[tab].page.SelectedID = strconv.Itoa(index)
 		}
 	})
 
@@ -179,3 +186,35 @@ func (t *tab) applyBottomBar() {
 	bottomBar.SetLabel(t.barLabel)
 	bottomBar.SetText(t.barText)
 }
+
+// clearSelected turns off any selection that was going on.
+// It does not affect the bottomBar.
+func (t *tab) clearSelected() {
+	t.page.Mode = structs.ModeOff
+	t.page.Selected = ""
+	t.page.SelectedID = ""
+	t.view.Highlight("")
+}
+
+// applySelected selects whatever is stored as the selected element in the struct,
+// and sets the mode accordingly.
+// It is safe to call if nothing was selected previously.
+//
+// applyBottomBar should be called after, as this func might set some bottomBar values.
+func (t *tab) applySelected() {
+	if t.page.Mode == structs.ModeOff {
+		// Just in case
+		t.page.Selected = ""
+		t.page.SelectedID = ""
+		t.view.Highlight("")
+		return
+	} else if t.page.Mode == structs.ModeLinkSelect {
+		t.view.Highlight(t.page.SelectedID).ScrollToHighlight()
+
+		if t.mode == tabModeDone {
+			// Page is not loading so bottomBar can change
+			t.barLabel = "[::b]Link: [::-]"
+			t.barText = t.page.Selected
+		}
+	}
+}

structs/structs.go

@@ -7,21 +7,31 @@ const (
 	TextPlain  Mediatype = "text/plain"
 )
 
+type PageMode int
+
+const (
+	ModeOff        PageMode = iota // Regular mode
+	ModeLinkSelect                 // When the enter key is pressed, allow for tab-based link navigation
+)
+
 // Page is for storing UTF-8 text/gemini pages, as well as text/plain pages.
 type Page struct {
-	Url       string
-	Mediatype Mediatype
-	Raw       string   // The raw response, as received over the network
-	Content   string   // The processed content, NOT raw. Uses cview colour tags. All link/link texts must have region tags. It will also have a left margin.
-	Links     []string // URLs, for each region in the content.
-	Row       int      // Scroll position
-	Column    int      // ditto
-	Width     int      // The width of the terminal at the time when the Content was set. This is to know when reformatting should happen.
+	Url        string
+	Mediatype  Mediatype
+	Raw        string   // The raw response, as received over the network
+	Content    string   // The processed content, NOT raw. Uses cview colour tags. All link/link texts must have region tags. It will also have a left margin.
+	Links      []string // URLs, for each region in the content.
+	Row        int      // Scroll position
+	Column     int      // ditto
+	Width      int      // The width of the terminal at the time when the Content was set. This is to know when reformatting should happen.
+	Selected   string   // The current text or link selected
+	SelectedID string   // The cview region ID for the selected text/link
+	Mode       PageMode
 }
 
 // Size returns an approx. size of a Page in bytes.
 func (p *Page) Size() int {
-	b := len(p.Raw) + len(p.Content) + len(p.Url)
+	b := len(p.Raw) + len(p.Content) + len(p.Url) + len(p.Selected) + len(p.SelectedID)
 	for i := range p.Links {
 		b += len(p.Links[i])
 	}

structs/structs_test.go

@@ -1,17 +0,0 @@
-package structs
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-)
-
-func TestSize(t *testing.T) {
-	p := Page{
-		Url:     "12345",
-		Raw:     "12345",
-		Content: "12345",
-		Links:   []string{"1", "2", "3", "4", "5"},
-	}
-	assert.Equal(t, 20, p.Size(), "sizes should be equal")
-}