Replace left margin spaces with an empty primitive

This commit introduced flashing issues in the tab row when reformatting,
and even can panic in certain reformatting situations. These bugs have
been logged in the inital comment of the PR.

CHANGELOG.md

@@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - More reliable start, no more flash of unindented text, or text that stays unindented (#107)
 - Pages with ANSI resets don't use the terminal's default text and background colors (#107)
 - ANSI documents don't leak color into the left margin (#107)
+- Rendering very long documents is now ~96% faster, excluding gemtext parsing (#107)
 
 
 ## [1.7.2] - 2020-12-21

NOTES.md

@@ -4,9 +4,6 @@
 - URL for each tab should not be stored as a string - in the current code there's lots of reparsing the URL
 
 ## Upstream Bugs
-- Text background not reset on ANSI pages
-  - Filed [issue 25](https://gitlab.com/tslocum/cview/-/issues/25)
-  - Add some bold back into modal text after this is fixed
 - Bookmark keys aren't deleted, just set to `""`
   - Waiting on [this viper PR](https://github.com/spf13/viper/pull/519) to be merged
 - Help table cells aren't dynamically wrapped

display/bookmarks.go

@@ -124,7 +124,7 @@ func Bookmarks(t *tab) {
 		bkmkPageRaw += fmt.Sprintf("=> %s %s\r\n", keys[i], m[keys[i]])
 	}
 	// Render and display
-	content, links := renderer.RenderGemini(bkmkPageRaw, textWidth(), leftMargin(), false)
+	content, links := renderer.RenderGemini(bkmkPageRaw, textWidth(), false)
 	page := structs.Page{
 		Raw:       bkmkPageRaw,
 		Content:   content,

display/display.go

@@ -6,6 +6,7 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
+	"sync"
 
 	"github.com/gdamore/tcell/v2"
 	"github.com/makeworld-the-better-one/amfora/cache"
@@ -52,6 +53,9 @@ var layout = cview.NewFlex()
 var newTabPage structs.Page
 var versionPage structs.Page
 
+// Global mutex for changing the size of the left margin on all tabs.
+var reformatMu = sync.Mutex{}
+
 var App = cview.NewApplication()
 
 func Init(version, commit, builtBy string) {
@@ -59,7 +63,7 @@ func Init(version, commit, builtBy string) {
 		"# Amfora Version Info\n\nAmfora:   %s\nCommit:   %s\nBuilt by: %s",
 		version, commit, builtBy,
 	)
-	renderVersionContent, versionLinks := renderer.RenderGemini(versionContent, textWidth(), leftMargin(), false)
+	renderVersionContent, versionLinks := renderer.RenderGemini(versionContent, textWidth(), false)
 	versionPage = structs.Page{
 		Raw:       versionContent,
 		Content:   renderVersionContent,
@@ -78,10 +82,22 @@ func Init(version, commit, builtBy string) {
 
 		// Make sure the current tab content is reformatted when the terminal size changes
 		go func(t *tab) {
+			reformatMu.Lock()
+			// Lock the app to prevent screen updates until this is done, because calling
+			// browser.AddTab updates the display
+			for i := range tabs {
+				// Overwrite tabs with a new, differently sized, left margin
+				browser.AddTab(strconv.Itoa(i), makeTabLabel(strconv.Itoa(i+1)), makeContentLayout(tabs[i].view))
+				if tabs[i] == t {
+					// Reformat page ASAP, in the middle of loop
+					// TODO The per-tab mutext is unecessary if the global one is used
 					t.reformatMu.Lock() // Only one reformat job per tab
-			defer t.reformatMu.Unlock()
-			// Use the current tab, but don't affect other tabs if the user switches tabs
 					reformatPageAndSetView(t, t.page)
+					t.reformatMu.Unlock()
+				}
+			}
+			App.Draw()
+			reformatMu.Unlock()
 		}(tabs[curTab])
 	})
 
@@ -240,7 +256,7 @@ func Init(version, commit, builtBy string) {
 	// Render the default new tab content ONCE and store it for later
 	// This code is repeated in Reload()
 	newTabContent := getNewTabContent()
-	renderedNewTabContent, newTabLinks := renderer.RenderGemini(newTabContent, textWidth(), leftMargin(), false)
+	renderedNewTabContent, newTabLinks := renderer.RenderGemini(newTabContent, textWidth(), false)
 	newTabPage = structs.Page{
 		Raw:       newTabContent,
 		Content:   renderedNewTabContent,
@@ -436,7 +452,7 @@ func NewTab() {
 	tabs[curTab].addToHistory("about:newtab")
 	tabs[curTab].history.pos = 0 // Manually set as first page
 
-	browser.AddTab(strconv.Itoa(curTab), makeTabLabel(strconv.Itoa(curTab+1)), tabs[curTab].view)
+	browser.AddTab(strconv.Itoa(curTab), makeTabLabel(strconv.Itoa(curTab+1)), makeContentLayout(tabs[curTab].view))
 	browser.SetCurrentTab(strconv.Itoa(curTab))
 	App.SetFocus(tabs[curTab].view)
 
@@ -520,7 +536,7 @@ func Reload() {
 		// Re-render new tab, similar to Init()
 		newTabContent := getNewTabContent()
 		tmpTermW := termW
-		renderedNewTabContent, newTabLinks := renderer.RenderGemini(newTabContent, textWidth(), leftMargin(), false)
+		renderedNewTabContent, newTabLinks := renderer.RenderGemini(newTabContent, textWidth(), false)
 		newTabPage = structs.Page{
 			Raw:       newTabContent,
 			Content:   renderedNewTabContent,

display/file.go

@@ -59,7 +59,7 @@ func handleFile(u string) (*structs.Page, bool) {
 		}
 
 		if mimetype == "text/gemini" {
-			rendered, links := renderer.RenderGemini(string(content), textWidth(), leftMargin(), false)
+			rendered, links := renderer.RenderGemini(string(content), textWidth(), false)
 			page = &structs.Page{
 				Mediatype: structs.TextGemini,
 				URL:       u,
@@ -73,7 +73,7 @@ func handleFile(u string) (*structs.Page, bool) {
 				Mediatype: structs.TextPlain,
 				URL:       u,
 				Raw:       string(content),
-				Content:   renderer.RenderPlainText(string(content), leftMargin()),
+				Content:   renderer.RenderPlainText(string(content)),
 				Links:     []string{},
 				Width:     termW,
 			}
@@ -107,7 +107,7 @@ func createDirectoryListing(u string) (*structs.Page, bool) {
 		content += fmt.Sprintf("=> %s%s %s%s\n", f.Name(), separator, f.Name(), separator)
 	}
 
-	rendered, links := renderer.RenderGemini(content, textWidth(), leftMargin(), false)
+	rendered, links := renderer.RenderGemini(content, textWidth(), false)
 	page = &structs.Page{
 		Mediatype: structs.TextGemini,
 		URL:       u,

display/handlers.go

@@ -373,7 +373,7 @@ func handleURL(t *tab, u string, numRedirects int) (string, bool) {
 	res.Body = rr.NewRestartReader(res.Body)
 
 	if renderer.CanDisplay(res) {
-		page, err := renderer.MakePage(u, res, textWidth(), leftMargin(), usingProxy)
+		page, err := renderer.MakePage(u, res, textWidth(), usingProxy)
 		// Rendering may have taken a while, make sure tab is still valid
 		if !isValidTab(t) {
 			return ret("", false)

display/private.go

@@ -65,11 +65,11 @@ func reformatPage(p *structs.Page) {
 			strings.HasPrefix(p.URL, "file") {
 			proxied = false
 		}
-		rendered, _ = renderer.RenderGemini(p.Raw, textWidth(), leftMargin(), proxied)
+		rendered, _ = renderer.RenderGemini(p.Raw, textWidth(), proxied)
 	case structs.TextPlain:
-		rendered = renderer.RenderPlainText(p.Raw, leftMargin())
+		rendered = renderer.RenderPlainText(p.Raw)
 	case structs.TextAnsi:
-		rendered = renderer.RenderANSI(p.Raw, leftMargin())
+		rendered = renderer.RenderANSI(p.Raw)
 	default:
 		// Rendering this type is not implemented
 		return
@@ -89,6 +89,8 @@ func reformatPageAndSetView(t *tab, p *structs.Page) {
 	reformatPage(p)
 	t.view.SetText(p.Content)
 	t.applyScroll() // Go back to where you were, roughly
+
+	App.Draw()
 }
 
 // setPage displays a Page on the passed tab number.

display/subscriptions.go

@@ -149,7 +149,7 @@ func Subscriptions(t *tab, u string) string {
 		}
 	}
 
-	content, links := renderer.RenderGemini(rawPage, textWidth(), leftMargin(), false)
+	content, links := renderer.RenderGemini(rawPage, textWidth(), false)
 	page := structs.Page{
 		Raw:       rawPage,
 		Content:   content,
@@ -191,7 +191,7 @@ func ManageSubscriptions(t *tab, u string) {
 		)
 	}
 
-	content, links := renderer.RenderGemini(rawPage, textWidth(), leftMargin(), false)
+	content, links := renderer.RenderGemini(rawPage, textWidth(), false)
 	page := structs.Page{
 		Raw:       rawPage,
 		Content:   content,

display/util.go

@@ -13,6 +13,17 @@ import (
 
 // This file contains funcs that are small, self-contained utilities.
 
+// makeContentLayout returns a flex that contains the given TextView
+// along with the current correct left margin.
+func makeContentLayout(tv *cview.TextView) *cview.Flex {
+	// Create horizontal flex with the left margin as an empty space
+	flex := cview.NewFlex()
+	flex.SetDirection(cview.FlexColumn)
+	flex.AddItem(nil, leftMargin(), 0, false)
+	flex.AddItem(tv, 0, 1, true)
+	return flex
+}
+
 // makeTabLabel takes a string and adds spacing to it, making it
 // suitable for display as a tab label.
 func makeTabLabel(s string) string {

renderer/page.go

@@ -58,7 +58,7 @@ func CanDisplay(res *gemini.Response) bool {
 
 // MakePage creates a formatted, rendered Page from the given network response and params.
 // You must set the Page.Width value yourself.
-func MakePage(url string, res *gemini.Response, width, leftMargin int, proxied bool) (*structs.Page, error) {
+func MakePage(url string, res *gemini.Response, width int, proxied bool) (*structs.Page, error) {
 	if !CanDisplay(res) {
 		return nil, ErrCantDisplay
 	}
@@ -101,7 +101,7 @@ func MakePage(url string, res *gemini.Response, width, leftMargin int, proxied b
 	}
 
 	if mediatype == "text/gemini" {
-		rendered, links := RenderGemini(utfText, width, leftMargin, proxied)
+		rendered, links := RenderGemini(utfText, width, proxied)
 		return &structs.Page{
 			Mediatype:    structs.TextGemini,
 			RawMediatype: mediatype,
@@ -119,7 +119,7 @@ func MakePage(url string, res *gemini.Response, width, leftMargin int, proxied b
 				RawMediatype: mediatype,
 				URL:          url,
 				Raw:          utfText,
-				Content:      RenderANSI(utfText, leftMargin),
+				Content:      RenderANSI(utfText),
 				Links:        []string{},
 				MadeAt:       time.Now(),
 			}, nil
@@ -131,7 +131,7 @@ func MakePage(url string, res *gemini.Response, width, leftMargin int, proxied b
 			RawMediatype: mediatype,
 			URL:          url,
 			Raw:          utfText,
-			Content:      RenderPlainText(utfText, leftMargin),
+			Content:      RenderPlainText(utfText),
 			Links:        []string{},
 			MadeAt:       time.Now(),
 		}, nil

renderer/renderer.go

@@ -21,7 +21,7 @@ var ansiRegex = regexp.MustCompile(`\x1b\[[0-9;]*m`)
 
 // RenderANSI renders plain text pages containing ANSI codes.
 // Practically, it is used for the text/x-ansi.
-func RenderANSI(s string, leftMargin int) string {
+func RenderANSI(s string) string {
 	s = cview.Escape(s)
 	if viper.GetBool("a-general.color") && viper.GetBool("a-general.ansi") {
 		s = cview.TranslateANSI(s)
@@ -33,24 +33,15 @@ func RenderANSI(s string, leftMargin int) string {
 	} else {
 		s = ansiRegex.ReplaceAllString(s, "")
 	}
-	var shifted string
-	lines := strings.Split(s, "\n")
-	for i := range lines {
-		shifted += fmt.Sprintf("[-:%s]", config.GetColorString("bg")) + strings.Repeat(" ", leftMargin) + lines[i] + "\n"
-	}
-	return shifted
+	return s
 }
 
 // RenderPlainText should be used to format plain text pages.
-func RenderPlainText(s string, leftMargin int) string {
-	var shifted string
-	lines := strings.Split(cview.Escape(s), "\n")
-	for i := range lines {
-		shifted += strings.Repeat(" ", leftMargin) +
-			"[" + config.GetColorString("regular_text") + "]" + lines[i] + "[-]" +
-			"\n"
-	}
-	return shifted
+func RenderPlainText(s string) string {
+	// It used to add a left margin, now this is done elsewhere.
+	// The function is kept for convenience and in case rendering
+	// is needed in the future.
+	return s
 }
 
 // wrapLine wraps a line to the provided width, and adds the provided prefix and suffix to each wrapped line.
@@ -283,7 +274,7 @@ func convertRegularGemini(s string, numLinks, width int, proxied bool) (string,
 //
 // proxied is whether the request is through the gemini:// scheme.
 // If it's not a gemini:// page, set this to true.
-func RenderGemini(s string, width, leftMargin int, proxied bool) (string, []string) {
+func RenderGemini(s string, width int, proxied bool) (string, []string) {
 	s = cview.Escape(s)
 
 	lines := strings.Split(s, "\n")
@@ -357,13 +348,5 @@ func RenderGemini(s string, width, leftMargin int, proxied bool) (string, []stri
 		processRegular()
 	}
 
-	if leftMargin > 0 {
-		renLines := strings.Split(rendered, "\n")
-		for i := range renLines {
-			renLines[i] = strings.Repeat(" ", leftMargin) + renLines[i]
-		}
-		return strings.Join(renLines, "\n"), links
-	}
-
 	return rendered, links
 }