starred/tview
1
0
Fork 0
mirror of https://github.com/rivo/tview.git synced 2026-04-26 21:35:54 +03:00
Code Issues 133 Projects Releases 1 Packages Wiki Activity

[PR #339] [CLOSED] Add escaping text transformer #915

New issue
Closed
opened 2026-03-04 01:08:32 +03:00 by kerem · 0 comments
kerem commented 2026-03-04 01:08:32 +03:00
Owner
Copy link

📋 Pull Request Information

Original PR: https://github.com/rivo/tview/pull/339
Author: @SamWhited
Created: 8/25/2019
Status: ❌ Closed

Base: master ← Head: escape_transformer

📝 Commits (1)

  • 84ad5f3 Add escaping text transformer

📊 Changes

3 files changed (+58 additions, -12 deletions)

View changed files

📝 go.mod (+2 -1)
📝 go.sum (+2 -4)
📝 util.go (+54 -7)

📄 Description

In an application I am populating a text view that uses colors from a file and do not want to buffer the entire file in memory multiple times or wait for the regexp to run over the entire file (which needs to be escaped to ensure that the user cannot inject colors or regions into the view).
Since text views are writers and the file is being copied in, I needed a wrapper that could wrap the writer and perform the escaping on the entire file as it was copied. A very nice API for dealing with text transformations like this already exists in the golang.org/x/text/transform package, including a way to apply the transformer to a writer. I originally wrote this for my package, but thought it might be useful for others as well and decided to submit it here.

This patch adds a transformer that can be used to escape text and re-implements the Escape function in terms of this transformer (resulting in a roughly 2x speedup on simple benchmarks). To keep the code simple and avoid having to redo all the standard transformer logic, the github.com/mpvl/textutil package was used. This means there is the potential to remove the dependency on textutil in the future and possibly further increase the speed of the escaper at the cost of lots more boilerplate in this package.

I did not include tests in this PR because on a previous change you told me that you didn't want them, however, the tests that I used to verify that the changes matched the old behavior are below. I would by very happy to include them in the PR if you change your mind about testing.

Thanks for your consideration!

Benchmark results 
$ go test -v -bench . -benchmem
=== RUN   TestEscape
=== RUN   TestEscape/0
=== RUN   TestEscape/0/Legacy
=== RUN   TestEscape/0/Transform
=== RUN   TestEscape/1
=== RUN   TestEscape/1/Legacy
=== RUN   TestEscape/1/Transform
=== RUN   TestEscape/2
=== RUN   TestEscape/2/Legacy
=== RUN   TestEscape/2/Transform
=== RUN   TestEscape/3
=== RUN   TestEscape/3/Legacy
=== RUN   TestEscape/3/Transform
=== RUN   TestEscape/4
=== RUN   TestEscape/4/Legacy
=== RUN   TestEscape/4/Transform
--- PASS: TestEscape (0.00s)
    --- PASS: TestEscape/0 (0.00s)
        --- PASS: TestEscape/0/Legacy (0.00s)
        --- PASS: TestEscape/0/Transform (0.00s)
    --- PASS: TestEscape/1 (0.00s)
        --- PASS: TestEscape/1/Legacy (0.00s)
        --- PASS: TestEscape/1/Transform (0.00s)
    --- PASS: TestEscape/2 (0.00s)
        --- PASS: TestEscape/2/Legacy (0.00s)
        --- PASS: TestEscape/2/Transform (0.00s)
    --- PASS: TestEscape/3 (0.00s)
        --- PASS: TestEscape/3/Legacy (0.00s)
        --- PASS: TestEscape/3/Transform (0.00s)
    --- PASS: TestEscape/4 (0.00s)
        --- PASS: TestEscape/4/Legacy (0.00s)
        --- PASS: TestEscape/4/Transform (0.00s)
goos: linux
goarch: amd64
pkg: github.com/rivo/tview
BenchmarkLegacy-8         397126              2676 ns/op             379 B/op         12 allocs/op
BenchmarkEscape-8         825540              1352 ns/op             434 B/op          5 allocs/op
BenchmarkTransform-8      887028              1240 ns/op             304 B/op          2 allocs/op
PASS
ok      github.com/rivo/tview   3.345s
escape_test.go 
package tview_test

import (
	"regexp"
	"strconv"
	"testing"

	"github.com/rivo/tview"
	"golang.org/x/text/transform"
)

var nonEscapePattern = regexp.MustCompile(`(\[[a-zA-Z0-9_,;: \-\."#]+\[*)\]`)

func legacyEscape(text string) string {
	return nonEscapePattern.ReplaceAllString(text, "$1[]")
}

var escapeTests = [...]struct {
	in, out string
}{
	0: {},
	1: {in: `["abc"][""][][red]`, out: `["abc"[][""[][][red[]`},
	2: {in: `[""[[[]`, out: `[""[[[[]`},
	3: {in: `["a[bc"]`, out: `["a[bc"[]`},
	4: {in: `["a]bc"]`, out: `["a[]bc"]`},
}

func TestEscape(t *testing.T) {
	for i, tc := range escapeTests {
		t.Run(strconv.Itoa(i), func(t *testing.T) {
			t.Run("Legacy", func(t *testing.T) {
				out := legacyEscape(tc.in)
				if out != tc.out {
					t.Errorf("want=`%s`, got=`%s`", tc.out, out)
				}
			})
			t.Run("Transform", func(t *testing.T) {
				et := tview.EscapeTransformer()
				out, _, err := transform.String(et, tc.in)
				if err != nil {
					t.Errorf("Unexpected error: %v", err)
				}
				if out != tc.out {
					t.Errorf("want=`%s`, got=`%s`", tc.out, out)
				}
			})
		})
	}
}

const benchEscape = `["abc"][""][][red][""[[[]["a[bc"]["a]bc"]`

func BenchmarkLegacy(b *testing.B) {
	for i := 0; i < b.N; i++ {
		_ = legacyEscape(benchEscape)
	}
}

func BenchmarkEscape(b *testing.B) {
	for i := 0; i < b.N; i++ {
		_ = tview.Escape(benchEscape)
	}
}

func BenchmarkTransform(b *testing.B) {
	t := tview.EscapeTransformer()
	b.ResetTimer()
	for i := 0; i < b.N; i++ {
		_, _, _ = transform.String(t, benchEscape)
	}
}

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/rivo/tview/pull/339 **Author:** [@SamWhited](https://github.com/SamWhited) **Created:** 8/25/2019 **Status:** ❌ Closed **Base:** `master` ← **Head:** `escape_transformer` --- ### 📝 Commits (1) - [`84ad5f3`](https://github.com/rivo/tview/commit/84ad5f3097b61ee1832a3d6f6284a6111448ade2) Add escaping text transformer ### 📊 Changes **3 files changed** (+58 additions, -12 deletions) <details> <summary>View changed files</summary> 📝 `go.mod` (+2 -1) 📝 `go.sum` (+2 -4) 📝 `util.go` (+54 -7) </details> ### 📄 Description In an application I am populating a text view that uses colors from a file and do not want to buffer the entire file in memory multiple times or wait for the regexp to run over the entire file (which needs to be escaped to ensure that the user cannot inject colors or regions into the view). Since text views are writers and the file is being copied in, I needed a wrapper that could wrap the writer and perform the escaping on the entire file as it was copied. A very nice API for dealing with text transformations like this already exists in the [`golang.org/x/text/transform`](https://godoc.org/golang.org/x/text/transform) package, including a way to apply the transformer to a writer. I originally wrote this for my package, but thought it might be useful for others as well and decided to submit it here. This patch adds a transformer that can be used to escape text and re-implements the `Escape` function in terms of this transformer (resulting in a roughly 2x speedup on simple benchmarks). To keep the code simple and avoid having to redo all the standard transformer logic, the [`github.com/mpvl/textutil`](https://godoc.org/github.com/mpvl/textutil) package was used. This means there is the potential to remove the dependency on textutil in the future and possibly further increase the speed of the escaper at the cost of lots more boilerplate in this package. I did not include tests in this PR because on a previous change you told me that you didn't want them, however, the tests that I used to verify that the changes matched the old behavior are below. I would by very happy to include them in the PR if you change your mind about testing. Thanks for your consideration! <details> <summary>Benchmark results</summary> <pre> $ go test -v -bench . -benchmem === RUN TestEscape === RUN TestEscape/0 === RUN TestEscape/0/Legacy === RUN TestEscape/0/Transform === RUN TestEscape/1 === RUN TestEscape/1/Legacy === RUN TestEscape/1/Transform === RUN TestEscape/2 === RUN TestEscape/2/Legacy === RUN TestEscape/2/Transform === RUN TestEscape/3 === RUN TestEscape/3/Legacy === RUN TestEscape/3/Transform === RUN TestEscape/4 === RUN TestEscape/4/Legacy === RUN TestEscape/4/Transform --- PASS: TestEscape (0.00s) --- PASS: TestEscape/0 (0.00s) --- PASS: TestEscape/0/Legacy (0.00s) --- PASS: TestEscape/0/Transform (0.00s) --- PASS: TestEscape/1 (0.00s) --- PASS: TestEscape/1/Legacy (0.00s) --- PASS: TestEscape/1/Transform (0.00s) --- PASS: TestEscape/2 (0.00s) --- PASS: TestEscape/2/Legacy (0.00s) --- PASS: TestEscape/2/Transform (0.00s) --- PASS: TestEscape/3 (0.00s) --- PASS: TestEscape/3/Legacy (0.00s) --- PASS: TestEscape/3/Transform (0.00s) --- PASS: TestEscape/4 (0.00s) --- PASS: TestEscape/4/Legacy (0.00s) --- PASS: TestEscape/4/Transform (0.00s) goos: linux goarch: amd64 pkg: github.com/rivo/tview BenchmarkLegacy-8 397126 2676 ns/op 379 B/op 12 allocs/op BenchmarkEscape-8 825540 1352 ns/op 434 B/op 5 allocs/op BenchmarkTransform-8 887028 1240 ns/op 304 B/op 2 allocs/op PASS ok github.com/rivo/tview 3.345s </pre> </details> <details> <summary>escape_test.go</summary> ```go package tview_test import ( "regexp" "strconv" "testing" "github.com/rivo/tview" "golang.org/x/text/transform" ) var nonEscapePattern = regexp.MustCompile(`(\[[a-zA-Z0-9_,;: \-\."#]+\[*)\]`) func legacyEscape(text string) string { return nonEscapePattern.ReplaceAllString(text, "$1[]") } var escapeTests = [...]struct { in, out string }{ 0: {}, 1: {in: `["abc"][""][][red]`, out: `["abc"[][""[][][red[]`}, 2: {in: `[""[[[]`, out: `[""[[[[]`}, 3: {in: `["a[bc"]`, out: `["a[bc"[]`}, 4: {in: `["a]bc"]`, out: `["a[]bc"]`}, } func TestEscape(t *testing.T) { for i, tc := range escapeTests { t.Run(strconv.Itoa(i), func(t *testing.T) { t.Run("Legacy", func(t *testing.T) { out := legacyEscape(tc.in) if out != tc.out { t.Errorf("want=`%s`, got=`%s`", tc.out, out) } }) t.Run("Transform", func(t *testing.T) { et := tview.EscapeTransformer() out, _, err := transform.String(et, tc.in) if err != nil { t.Errorf("Unexpected error: %v", err) } if out != tc.out { t.Errorf("want=`%s`, got=`%s`", tc.out, out) } }) }) } } const benchEscape = `["abc"][""][][red][""[[[]["a[bc"]["a]bc"]` func BenchmarkLegacy(b *testing.B) { for i := 0; i < b.N; i++ { _ = legacyEscape(benchEscape) } } func BenchmarkEscape(b *testing.B) { for i := 0; i < b.N; i++ { _ = tview.Escape(benchEscape) } } func BenchmarkTransform(b *testing.B) { t := tview.EscapeTransformer() b.ResetTimer() for i := 0; i < b.N; i++ { _, _, _ = transform.String(t, benchEscape) } } ``` </details> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
kerem 2026-03-04 01:08:32 +03:00
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
master
image
revert-759-textarea
v0.42.0
Labels
Clear labels   
announcement

   
on roadmap

   
pull-request

Mirrored from GitHub Pull Request

  
requesting feedback

   
will consider
No labels
announcement
on roadmap
pull-request
requesting feedback
will consider
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/tview#915
No description provided.