From 90654cd2cfdc4e6cf7bb2cdd35b1171c4d7807e9 Mon Sep 17 00:00:00 2001 From: makeworld Date: Tue, 29 Dec 2020 17:35:29 -0500 Subject: [PATCH] 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 | 1 + NOTES.md | 3 --- display/bookmarks.go | 2 +- display/display.go | 32 ++++++++++++++++++++++++-------- display/file.go | 6 +++--- display/handlers.go | 2 +- display/private.go | 8 +++++--- display/subscriptions.go | 4 ++-- display/util.go | 11 +++++++++++ renderer/page.go | 8 ++++---- renderer/renderer.go | 33 ++++++++------------------------- 11 files changed, 60 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0afc283..9bc0849 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/NOTES.md b/NOTES.md index 9c6d7ac..5af1b1e 100644 --- a/NOTES.md +++ b/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 diff --git a/display/bookmarks.go b/display/bookmarks.go index 9a73b07..5062582 100644 --- a/display/bookmarks.go +++ b/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, diff --git a/display/display.go b/display/display.go index 63e4cd0..34ea158 100644 --- a/display/display.go +++ b/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) { - 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) + 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 + 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, diff --git a/display/file.go b/display/file.go index 9349c86..a5ba3da 100644 --- a/display/file.go +++ b/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, diff --git a/display/handlers.go b/display/handlers.go index e9234df..9ed4add 100644 --- a/display/handlers.go +++ b/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) diff --git a/display/private.go b/display/private.go index 403e884..0bd7825 100644 --- a/display/private.go +++ b/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. diff --git a/display/subscriptions.go b/display/subscriptions.go index 5e74127..d65c19e 100644 --- a/display/subscriptions.go +++ b/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, diff --git a/display/util.go b/display/util.go index fb8473b..ff0888b 100644 --- a/display/util.go +++ b/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 { diff --git a/renderer/page.go b/renderer/page.go index 0c43306..9c637bd 100644 --- a/renderer/page.go +++ b/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 diff --git a/renderer/renderer.go b/renderer/renderer.go index cd9d09b..9552835 100644 --- a/renderer/renderer.go +++ b/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 }