🚧 Switch to using tab pointers instead of ints

Almost finished overall work.

CHANGELOG.md

@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 - Many potential network and display race conditions eliminated
 - Whether a tab is loading stays indicated when you switch away from it and go back
+- Plain text documents are displayed faithfully (there were some edge conditions)
 
 ## [1.2.0] - 2020-07-02
 ### Added

NOTES.md

@@ -1,12 +1,9 @@
 # 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

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(tab int) {
+func Bookmarks(t *tab) {
 	// Gather bookmarks
 	rawContent := "# Bookmarks\r\n\r\n"
 	m, keys := bookmarks.All()
@@ -113,8 +113,8 @@ func Bookmarks(tab int) {
 		Width:     termW,
 		Mediatype: structs.TextGemini,
 	}
-	setPage(curTab, &page)
+	setPage(t, &page)
-	tabs[tab].applyBottomBar()
+	t.applyBottomBar()
 }
 
 // addBookmark goes through the process of adding a bookmark for the current page.

display/display.go

@@ -65,7 +65,7 @@ var layout = cview.NewFlex().
 
 var renderedNewTabContent string
 var newTabLinks []string
-var newTabPage *structs.Page
+var newTabPage structs.Page
 
 var App = cview.NewApplication().
 	EnableMouse(false).
@@ -76,12 +76,12 @@ var App = cview.NewApplication().
 		termH = height
 
 		// Make sure the current tab content is reformatted when the terminal size changes
-		go func(tab int) {
+		go func(t *tab) {
-			tabs[tab].reformatMut.Lock() // Only one reformat job per tab
+			t.reformatMut.Lock() // Only one reformat job per tab
-			defer tabs[tab].reformatMut.Unlock()
+			defer t.reformatMut.Unlock()
 			// Use the current tab, but don't affect other tabs if the user switches tabs
-			reformatPageAndSetView(tab, tabs[tab].page)
+			reformatPageAndSetView(t, t.page)
-		}(curTab)
+		}(tabs[curTab])
 	})
 
 func Init() {
@@ -180,7 +180,7 @@ func Init() {
 			}
 			if i <= len(tabs[tab].page.Links) && i > 0 {
 				// It's a valid link number
-				followLink(tab, tabs[tab].page.Url, tabs[tab].page.Links[i-1])
+				followLink(tabs[tab], tabs[tab].page.Url, tabs[tab].page.Links[i-1])
 				return
 			}
 			// Invalid link number, don't do anything
@@ -199,7 +199,7 @@ func Init() {
 
 	// Render the default new tab content ONCE and store it for later
 	renderedNewTabContent, newTabLinks = renderer.RenderGemini(newTabContent, textWidth(), leftMargin())
-	newTabPage = &structs.Page{
+	newTabPage = structs.Page{
 		Raw:       newTabContent,
 		Content:   renderedNewTabContent,
 		Links:     newTabLinks,
@@ -230,11 +230,11 @@ func Init() {
 			// History arrow keys
 			if event.Modifiers() == tcell.ModAlt {
 				if event.Key() == tcell.KeyLeft {
-					histBack()
+					histBack(tabs[curTab])
 					return nil
 				}
 				if event.Key() == tcell.KeyRight {
-					histForward()
+					histForward(tabs[curTab])
 					return nil
 				}
 			}
@@ -242,7 +242,7 @@ func Init() {
 			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)
+					next, err := resolveRelLink(tabs[curTab], tabs[curTab].page.Url, tabs[curTab].page.Selected)
 					if err != nil {
 						Error("URL Error", err.Error())
 						return nil
@@ -260,7 +260,7 @@ func Init() {
 				URL(viper.GetString("a-general.home"))
 				return nil
 			case tcell.KeyCtrlB:
-				Bookmarks(curTab)
+				Bookmarks(tabs[curTab])
 				tabs[curTab].addToHistory("about:bookmarks")
 				return nil
 			case tcell.KeyCtrlD:
@@ -286,10 +286,10 @@ func Init() {
 					Reload()
 					return nil
 				case "b":
-					histBack()
+					histBack(tabs[curTab])
 					return nil
 				case "f":
-					histForward()
+					histForward(tabs[curTab])
 					return nil
 				case "u":
 					tabs[curTab].pageUp()
@@ -383,7 +383,8 @@ func NewTab() {
 	curTab = NumTabs()
 
 	tabs = append(tabs, makeNewTab())
-	setPage(curTab, newTabPage)
+	temp := newTabPage // Copy
+	setPage(tabs[curTab], &temp)
 
 	// Can't go backwards, but this isn't the first page either.
 	// The first page will be the next one the user goes to.
@@ -479,7 +480,7 @@ func SwitchTab(tab int) {
 	curTab = tab % NumTabs()
 
 	// Display tab
-	reformatPageAndSetView(curTab, tabs[curTab].page)
+	reformatPageAndSetView(tabs[curTab], tabs[curTab].page)
 	tabPages.SwitchToPage(strconv.Itoa(curTab))
 	tabRow.Highlight(strconv.Itoa(curTab)).ScrollToHighlight()
 	tabs[curTab].applySelected()
@@ -497,13 +498,13 @@ func Reload() {
 	}
 
 	cache.Remove(tabs[curTab].page.Url)
-	go func(tab int) {
+	go func(t *tab) {
-		handleURL(tab, tabs[tab].page.Url) // goURL is not used bc history shouldn't be added to
+		handleURL(t, t.page.Url) // goURL is not used bc history shouldn't be added to
-		if tab == curTab {
+		if t == tabs[curTab] {
 			// Display the bottomBar state that handleURL set
-			tabs[tab].applyBottomBar()
+			t.applyBottomBar()
 		}
-	}(curTab)
+	}(tabs[curTab])
 }
 
 // URL loads and handles the provided URL for the current tab.
@@ -512,12 +513,13 @@ func URL(u string) {
 	// Some code is copied in followLink()
 
 	if u == "about:bookmarks" {
-		Bookmarks(curTab)
+		Bookmarks(tabs[curTab])
 		tabs[curTab].addToHistory("about:bookmarks")
 		return
 	}
 	if u == "about:newtab" {
-		setPage(curTab, newTabPage)
+		temp := newTabPage // Copy
+		setPage(tabs[curTab], &temp)
 		return
 	}
 	if strings.HasPrefix(u, "about:") {
@@ -525,7 +527,7 @@ func URL(u string) {
 		return
 	}
 
-	go goURL(curTab, u)
+	go goURL(tabs[curTab], u)
 }
 
 func NumTabs() int {

display/history.go

@@ -1,35 +1,35 @@
 package display
 
-func histForward() {
+func histForward(t *tab) {
-	if tabs[curTab].history.pos >= len(tabs[curTab].history.urls)-1 {
+	if t.history.pos >= len(t.history.urls)-1 {
 		// Already on the most recent URL in the history
 		return
 	}
-	tabs[curTab].history.pos++
+	t.history.pos++
-	go func(tab int) {
+	go func(tt *tab) {
-		handleURL(tab, tabs[tab].history.urls[tabs[tab].history.pos]) // Load that position in history
+		handleURL(tt, tt.history.urls[tt.history.pos]) // Load that position in history
-		tabs[tab].applyScroll()
+		tt.applyScroll()
-		tabs[tab].applySelected()
+		tt.applySelected()
-		if tab == curTab {
+		if tt == tabs[curTab] {
 			// Display the bottomBar state that handleURL set
-			tabs[tab].applyBottomBar()
+			tt.applyBottomBar()
 		}
-	}(curTab)
+	}(t)
 }
 
-func histBack() {
+func histBack(t *tab) {
-	if tabs[curTab].history.pos <= 0 {
+	if t.history.pos <= 0 {
 		// First tab in history
 		return
 	}
-	tabs[curTab].history.pos--
+	t.history.pos--
-	go func(tab int) {
+	go func(tt *tab) {
-		handleURL(tab, tabs[tab].history.urls[tabs[tab].history.pos]) // Load that position in history
+		handleURL(tt, tt.history.urls[tt.history.pos]) // Load that position in history
-		tabs[tab].applyScroll()
+		tt.applyScroll()
-		tabs[tab].applySelected()
+		tt.applySelected()
-		if tab == curTab {
+		if tt == tabs[curTab] {
 			// Display the bottomBar state that handleURL set
-			tabs[tab].applyBottomBar()
+			tt.applyBottomBar()
 		}
-	}(curTab)
+	}(t)
 }

display/private.go

@@ -18,6 +18,16 @@ import (
 
 // This file contains the functions that aren't part of the public API.
 
+// isValidTab indicates whether the passed tab is still being used, even if it's not currently displayed.
+func isValidTab(t *tab) bool {
+	for i := range tabs {
+		if tabs[i] == t {
+			return true
+		}
+	}
+	return false
+}
+
 func leftMargin() int {
 	return int(float64(termW) * viper.GetFloat64("a-general.left_margin"))
 }
@@ -51,8 +61,8 @@ func queryEscape(path string) string {
 // resolveRelLink returns an absolute link for the given absolute link and relative one.
 // It also returns an error if it could not resolve the links, which should be displayed
 // to the user.
-func resolveRelLink(tab int, prev, next string) (string, error) {
+func resolveRelLink(t *tab, prev, next string) (string, error) {
-	if !tabs[tab].hasContent() {
+	if !t.hasContent() {
 		return next, nil
 	}
 
@@ -67,12 +77,12 @@ func resolveRelLink(tab int, prev, next string) (string, error) {
 // followLink should be used when the user "clicks" a link on a page.
 // Not when a URL is opened on a new tab for the first time.
 // It will handle setting the bottomBar.
-func followLink(tab int, prev, next string) {
+func followLink(t *tab, prev, next string) {
 
 	// Copied from URL()
 	if next == "about:bookmarks" {
-		Bookmarks(tab)
+		Bookmarks(t)
-		tabs[tab].addToHistory("about:bookmarks")
+		t.addToHistory("about:bookmarks")
 		return
 	}
 	if strings.HasPrefix(next, "about:") {
@@ -80,14 +90,14 @@ func followLink(tab int, prev, next string) {
 		return
 	}
 
-	if tabs[tab].hasContent() {
+	if t.hasContent() {
-		tabs[tab].saveScroll() // Likely called later on, it's here just in case
+		t.saveScroll() // Likely called later on, it's here just in case
-		nextURL, err := resolveRelLink(tab, prev, next)
+		nextURL, err := resolveRelLink(t, prev, next)
 		if err != nil {
 			Error("URL Error", err.Error())
 			return
 		}
-		go goURL(tab, nextURL)
+		go goURL(t, nextURL)
 		return
 	}
 	// No content on current tab, so the "prev" URL is not valid.
@@ -97,7 +107,7 @@ func followLink(tab int, prev, next string) {
 		Error("URL Error", "Link URL could not be parsed")
 		return
 	}
-	go goURL(tab, next)
+	go goURL(t, next)
 }
 
 // reformatPage will take the raw page content and reformat it according to the current terminal dimensions.
@@ -126,33 +136,38 @@ func reformatPage(p *structs.Page) {
 
 // reformatPageAndSetView is for reformatting a page that is already being displayed.
 // setPage should be used when a page is being loaded for the first time.
-func reformatPageAndSetView(tab int, p *structs.Page) {
+func reformatPageAndSetView(t *tab, p *structs.Page) {
-	tabs[tab].saveScroll()
+	t.saveScroll()
 	reformatPage(p)
-	tabs[tab].view.SetText(p.Content)
+	t.view.SetText(p.Content)
-	tabs[tab].applyScroll() // Go back to where you were, roughly
+	t.applyScroll() // Go back to where you were, roughly
 }
 
 // setPage displays a Page on the passed tab number.
 // The bottomBar is not actually changed in this func
-func setPage(tab int, p *structs.Page) {
+func setPage(t *tab, p *structs.Page) {
-	tabs[tab].saveScroll() // Save the scroll of the previous page
+	if !isValidTab(t) {
+		// Don't waste time reformatting an invalid tab
+		return
+	}
+
+	t.saveScroll() // Save the scroll of the previous page
 
 	// Make sure the page content is fitted to the terminal every time it's displayed
 	reformatPage(p)
 
 	// Change page on screen
-	tabs[tab].page = p
+	t.page = p
-	tabs[tab].view.SetText(p.Content)
+	t.view.SetText(p.Content)
-	tabs[tab].view.Highlight("") // Turn off highlights
+	t.view.Highlight("") // Turn off highlights, other funcs may restore if necessary
-	tabs[tab].view.ScrollToBeginning()
+	t.view.ScrollToBeginning()
 
 	// Setup display
-	App.SetFocus(tabs[tab].view)
+	App.SetFocus(t.view)
 
 	// Save bottom bar for the tab - TODO: other funcs will apply/display it
-	tabs[tab].barLabel = ""
+	t.barLabel = ""
-	tabs[tab].barText = p.Url
+	t.barText = p.Url
 }
 
 // goURL is like handleURL, but takes care of history and the bottomBar.
@@ -160,14 +175,14 @@ func setPage(tab int, p *structs.Page) {
 // It has no return values to be processed.
 //
 // It should be called in a goroutine.
-func goURL(tab int, u string) {
+func goURL(t *tab, u string) {
-	final, displayed := handleURL(tab, u)
+	final, displayed := handleURL(t, u)
 	if displayed {
-		tabs[tab].addToHistory(final)
+		t.addToHistory(final)
 	}
-	if tab == curTab {
+	if t == tabs[curTab] {
 		// Display the bottomBar state that handleURL set
-		tabs[tab].applyBottomBar()
+		t.applyBottomBar()
 	}
 }
 
@@ -183,15 +198,32 @@ func goURL(tab int, u string) {
 //
 // The bottomBar is not actually changed in this func, except during loading.
 // The func that calls this one should apply the bottomBar values if necessary.
-func handleURL(tab int, u string) (string, bool) {
+func handleURL(t *tab, u string) (string, bool) {
 	defer App.Draw() // Just in case
 
-	App.SetFocus(tabs[tab].view)
+	// Save for resetting on error
+	oldLable := t.barLabel
+	oldText := t.barText
+
+	// Custom return function
+	ret := func(s string, b bool) (string, bool) {
+		if !b {
+			// Reset bottomBar if page wasn't loaded
+			t.barLabel = oldLable
+			t.barText = oldText
+		}
+		return s, b
+	}
+
+	t.barLabel = ""
+	bottomBar.SetLabel("")
+
+	App.SetFocus(t.view)
 
 	// To allow linking to the bookmarks page, and history browsing
 	if u == "about:bookmarks" {
-		Bookmarks(tab)
+		Bookmarks(t)
-		return "about:bookmarks", true
+		return ret("about:bookmarks", true)
 	}
 
 	u = normalizeURL(u)
@@ -199,8 +231,7 @@ func handleURL(tab int, u string) (string, bool) {
 	parsed, err := url.Parse(u)
 	if err != nil {
 		Error("URL Error", err.Error())
-		tabs[tab].barText = tabs[tab].page.Url
+		return ret("", false)
-		return "", false
 	}
 
 	if strings.HasPrefix(u, "http") {
@@ -222,33 +253,36 @@ func handleURL(tab int, u string) (string, bool) {
 				Error("HTTP Error", "Error executing custom browser command: "+err.Error())
 			}
 		}
-		tabs[tab].barText = tabs[tab].page.Url
+		return ret("", false)
-		return "", false
 	}
 	if !strings.HasPrefix(u, "gemini") {
 		Error("Protocol Error", "Only gemini and HTTP are supported. URL was "+u)
-		tabs[tab].barText = tabs[tab].page.Url
+		return ret("", false)
-		return "", false
 	}
 	// Gemini URL
 
 	// Load page from cache if possible
 	page, ok := cache.Get(u)
 	if ok {
-		setPage(tab, page)
+		setPage(t, page)
-		return u, true
+		return ret(u, true)
 	}
 	// Otherwise download it
 	bottomBar.SetText("Loading...")
-	tabs[tab].barText = "Loading..." // Save it too, in case the tab switches during loading
+	t.barText = "Loading..." // Save it too, in case the tab switches during loading
-	tabs[tab].mode = tabModeLoading
+	t.mode = tabModeLoading
-	defer func(t int) {
+	defer func(tt *tab) {
-		tabs[t].mode = tabModeDone
+		tt.mode = tabModeDone
-	}(tab)
+	}(t)
 	App.Draw()
 
 	res, err := client.Fetch(u)
 
+	// Loading may have taken a while, make sure tab is still valid
+	if !isValidTab(t) {
+		return ret("", false)
+	}
+
 	if err == client.ErrTofu {
 		if Tofu(parsed.Host) {
 			// They want to continue anyway
@@ -256,37 +290,31 @@ func handleURL(tab int, u string) (string, bool) {
 			// Response can be used further down, no need to reload
 		} else {
 			// They don't want to continue
-			// Set the bar back to original URL
+			return ret("", false)
-			tabs[tab].barText = tabs[tab].page.Url
-			return "", false
 		}
 	} else if err != nil {
 		Error("URL Fetch Error", err.Error())
-		// Set the bar back to original URL
+		return ret("", false)
-		tabs[tab].barText = tabs[tab].page.Url
-		return "", false
 	}
 	if renderer.CanDisplay(res) {
 		page, err := renderer.MakePage(u, res, textWidth(), leftMargin())
+		// Rendering may have taken a while, make sure tab is still valid
+		if !isValidTab(t) {
+			return ret("", false)
+		}
+
 		page.Width = termW
 		if err != nil {
 			Error("Page Error", "Issuing creating page: "+err.Error())
-			// Set the bar back to original URL
+			return ret("", false)
-			tabs[tab].barText = tabs[tab].page.Url
-			return "", false
 		}
 		go cache.Add(page)
-		setPage(tab, page)
+		setPage(t, page)
-		return u, true
+		return ret(u, true)
 	}
 	// Not displayable
 	// Could be a non 20 (or 21) status code, or a different kind of document
 
-	// Set the bar back to original URL
-	bottomBar.SetText(tabs[curTab].page.Url)
-	tabs[tab].barText = tabs[curTab].page.Url
-	App.Draw()
-
 	// Handle each status code
 	switch gemini.SimplifyStatus(res.Status) {
 	case 10:
@@ -298,31 +326,31 @@ func handleURL(tab int, u string) (string, bool) {
 			if len(parsed.String()) > 1024 {
 				// 1024 is the max size for URLs in the spec
 				Error("Input Error", "URL for that input would be too long.")
-				return "", false
+				return ret("", false)
 			}
-			return handleURL(tab, parsed.String())
+			return ret(handleURL(t, parsed.String()))
 		}
-		return "", false
+		return ret("", false)
 	case 30:
 		parsedMeta, err := url.Parse(res.Meta)
 		if err != nil {
 			Error("Redirect Error", "Invalid URL: "+err.Error())
-			return "", false
+			return ret("", false)
 		}
 		redir := parsed.ResolveReference(parsedMeta).String()
 		if YesNo("Follow redirect?\n" + redir) {
-			return handleURL(tab, redir)
+			return handleURL(t, redir)
 		}
-		return "", false
+		return ret("", false)
 	case 40:
 		Error("Temporary Failure", cview.Escape(res.Meta))
-		return "", false
+		return ret("", false)
 	case 50:
 		Error("Permanent Failure", cview.Escape(res.Meta))
-		return "", false
+		return ret("", false)
 	case 60:
 		Info("The server requested a certificate. Cert handling is coming to Amfora soon!")
-		return "", false
+		return ret("", false)
 	}
 	// Status code 20, but not a document that can be displayed
 	yes := YesNo("This type of file can't be displayed. Downloading will be implemented soon. Would like to open the file in a HTTPS proxy for now?")
@@ -345,7 +373,7 @@ func handleURL(tab int, u string) (string, bool) {
 		}
 		App.Draw()
 	}
-	return "", false
+	return ret("", false)
 }
 
 // normalizeURL attempts to make URLs that are different strings

display/tab.go

@@ -79,7 +79,7 @@ func makeNewTab() *tab {
 				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])
+				followLink(tabs[tab], tabs[tab].page.Url, tabs[tab].page.Links[linkN])
 				return
 			} else {
 				// They've started link highlighting