Skip to content

Commit 67ea0b7

Browse files
yorukotyorukot
authored
Merge pull request #557 from lazysegtree/fix_preview
Fix previews for text file with control characters - Avoid layout breaking, improved code, added tests for string functions
2 parents 204954e + 16ca97a commit 67ea0b7

File tree

3 files changed

+146
-80
lines changed

3 files changed

+146
-80
lines changed

‎src/internal/model_render.go

Lines changed: 54 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,35 @@ func (m model) sortOptionsRender() string {
551551
return sortOptionsModalBorderStyle(panel.sortOptions.height, panel.sortOptions.width, bottomBorder).Render(sortOptionsContent)
552552
}
553553

554+
func readFileContent(filepath string, maxLineLength int, previewLine int) (string, error) {
555+
// String builder is much better for efficiency
556+
// See - https://stackoverflow.com/questions/1760757/how-to-efficiently-concatenate-strings-in-go/47798475#47798475
557+
var resultBuilder strings.Builder
558+
file, err := os.Open(filepath)
559+
if err != nil {
560+
return resultBuilder.String(), err
561+
}
562+
defer file.Close()
563+
564+
scanner := bufio.NewScanner(file)
565+
lineCount := 0
566+
for scanner.Scan() {
567+
line := scanner.Text()
568+
if len(line) > maxLineLength {
569+
line = line[:maxLineLength]
570+
}
571+
// This is critical to avoid layout break, removes non Printable ASCII control characters.
572+
line = makePrintable(line)
573+
resultBuilder.WriteString(line+"\n")
574+
lineCount++
575+
if previewLine > 0 && lineCount >= previewLine {
576+
break
577+
}
578+
}
579+
// returns the first non-EOF error that was encountered by the [Scanner]
580+
return resultBuilder.String(), scanner.Err()
581+
}
582+
554583
func (m model) filePreviewPanelRender() string {
555584
previewLine := m.mainPanelHeight + 2
556585
m.fileModel.filePreview.width += m.fullWidth - Config.SidebarWidth - m.fileModel.filePreview.width - ((m.fileModel.width + 2) * len(m.fileModel.filePanels)) - 2
@@ -630,86 +659,42 @@ func (m model) filePreviewPanelRender() string {
630659
}
631660

632661
format := lexers.Match(filepath.Base(itemPath))
633-
if format != nil {
634-
var codeHighlight string
635-
var err error
636-
var fileContent string
637-
file, err := os.Open(itemPath)
662+
663+
if format == nil {
664+
isText, err := isTextFile(itemPath)
638665
if err != nil {
639-
outPutLog(err)
640-
return box.Render("\n --- " + icon.Error + " Error open file ---")
666+
outPutLog("Error while checking text file", err)
667+
return box.Render("\n --- " + icon.Error + " Error get file info ---")
668+
} else if !isText {
669+
return box.Render("\n --- " + icon.Error + " Unsupported formats ---")
641670
}
642-
defer file.Close()
643-
644-
scanner := bufio.NewScanner(file)
645-
lineCount := 0
671+
}
646672

647-
maxLineLength := m.fileModel.width + 20
648-
for scanner.Scan() {
649-
line := scanner.Text()
650-
if len(line) > maxLineLength {
651-
line = line[:maxLineLength]
652-
}
653-
fileContent += line + "\n"
654-
lineCount++
655-
if previewLine > 0 && lineCount >= previewLine {
656-
break
657-
}
658-
}
673+
// At this point either format is not nil, or we can read the file
674+
fileContent , err := readFileContent(itemPath, m.fileModel.width+20, previewLine)
675+
if err != nil {
676+
outPutLog(err)
677+
return box.Render("\n --- " + icon.Error + " Error open file ---")
678+
}
659679

660-
if Config.TransparentBackground {
661-
codeHighlight, err = ansichroma.HightlightString(fileContent, format.Config().Name, theme.CodeSyntaxHighlightTheme, "")
662-
} else {
663-
codeHighlight, err = ansichroma.HightlightString(fileContent, format.Config().Name, theme.CodeSyntaxHighlightTheme, theme.FilePanelBG)
680+
// We know the format of file, and we can apply syntax highlighting
681+
if format != nil {
682+
background := ""
683+
if ! Config.TransparentBackground {
684+
background = theme.FilePanelBG
664685
}
686+
fileContent, err = ansichroma.HightlightString(fileContent, format.Config().Name, theme.CodeSyntaxHighlightTheme, background)
665687
if err != nil {
666688
outPutLog("Error render code highlight", err)
667689
return box.Render("\n --- " + icon.Error + " Error render code highlight ---")
668690
}
669-
if codeHighlight == "" {
670-
return box.Render("\n --- empty ---")
671-
}
672-
673-
codeHighlight = checkAndTruncateLineLengths(codeHighlight, m.fileModel.filePreview.width)
674-
675-
return box.Render(codeHighlight)
676-
} else {
677-
textFile, err := isTextFile(itemPath)
678-
if err != nil {
679-
outPutLog("Error check text file", err)
680-
}
681-
if textFile {
682-
var fileContent string
683-
file, err := os.Open(itemPath)
684-
if err != nil {
685-
outPutLog(err)
686-
return box.Render("\n --- " + icon.Error + " Error open file ---")
687-
}
688-
defer file.Close()
689-
690-
scanner := bufio.NewScanner(file)
691-
lineCount := 0
692-
693-
for scanner.Scan() {
694-
fileContent += scanner.Text() + "\n"
695-
lineCount++
696-
if previewLine > 0 && lineCount >= previewLine {
697-
break
698-
}
699-
}
700-
701-
if err := scanner.Err(); err != nil {
702-
outPutLog(err)
703-
return box.Render("\n --- " + icon.Error + " Error open file ---")
704-
}
705-
706-
textContent := checkAndTruncateLineLengths(fileContent, m.fileModel.filePreview.width)
707-
708-
return box.Render(textContent)
709-
}
710691
}
711692

712-
return box.Render("\n --- " + icon.Error + " Unsupported formats ---")
693+
if fileContent == "" {
694+
return box.Render("\n --- empty ---")
695+
}
696+
fileContent = checkAndTruncateLineLengths(fileContent, m.fileModel.filePreview.width)
697+
return box.Render(fileContent)
713698
}
714699

715700
func (m model) commandLineInputBoxRender() string {

‎src/internal/string_function.go

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package internal
33
import (
44
"bufio"
55
"fmt"
6+
"io"
67
"math"
78
"os"
89
"strings"
@@ -142,6 +143,17 @@ func checkAndTruncateLineLengths(text string, maxLength int) string {
142143
return finalResult
143144
}
144145

146+
// Separated this out out for easy testing
147+
func isBufferPrintable(buffer []byte) bool {
148+
for _, b := range buffer {
149+
// This will also handle b==0
150+
if !unicode.IsPrint(rune(b)) && !unicode.IsSpace(rune(b)) {
151+
return false
152+
}
153+
}
154+
return true
155+
}
156+
145157
// Check file is text file or not
146158
func isTextFile(filename string) (bool, error) {
147159
file, err := os.Open(filename)
@@ -152,19 +164,28 @@ func isTextFile(filename string) (bool, error) {
152164

153165
reader := bufio.NewReader(file)
154166
buffer := make([]byte, 1024)
155-
_, err = reader.Read(buffer)
156-
if err != nil {
167+
cnt, err := reader.Read(buffer)
168+
if err != nil && err != io.EOF {
157169
return false, err
158170
}
171+
return isBufferPrintable(buffer[:cnt]), nil
172+
}
159173

160-
for _, b := range buffer {
161-
if b == 0 {
162-
return false, nil
163-
}
164-
if !unicode.IsPrint(rune(b)) && !unicode.IsSpace(rune(b)) {
165-
return false, nil
174+
175+
// Although some characters like `\x0b`(vertical tab) are printable,
176+
// previewing them breaks the layout.
177+
// So, among the "non-graphic" printable characters, we only need \n and \t
178+
// Space and NBSP are already considered graphic by unicode.
179+
func makePrintable(line string)(string) {
180+
var sb strings.Builder
181+
// This has to be looped byte-wise, looping it rune-wise
182+
// or by using strings.Map would cause issues with strings like
183+
// "(NBSP)\xa0"
184+
for i:=0; i<len(line); i++ {
185+
r := rune(line[i])
186+
if unicode.IsGraphic(r) || r == rune('\t') || r == rune('\n') {
187+
sb.WriteByte(line[i])
166188
}
167189
}
168-
169-
return true, nil
170-
}
190+
return sb.String()
191+
}

‎src/internal/string_function_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,63 @@ func TestFilenameWithouText(t *testing.T) {
5757
})
5858
}
5959
}
60+
61+
62+
func TestIsBufferPrintable(t *testing.T) {
63+
var inputs = []struct {
64+
input string
65+
expected bool
66+
} {
67+
{"", true},
68+
{"hello", true},
69+
{"abcdABCD0123~!@#$%^&*()_+-={}|:\"<>?,./;'[]", true},
70+
{"Horizontal Tab and NewLine\t\t\n\n", true},
71+
{"\xa0(NBSP)", true},
72+
{"\x0b(Vertical Tab)", true},
73+
{"\x0d(CR)", true},
74+
{"ASCII control characters : \x00(NULL)", false},
75+
{"\x05(ENQ)", false},
76+
{"\x0f(SI)", false},
77+
{"\x1b(ESC)", false},
78+
{"\x7f(DEL)", false},
79+
}
80+
for _, tt := range inputs {
81+
82+
t.Run(fmt.Sprintf("Testing if buffer %q is printable", tt.input), func(t* testing.T){
83+
result := isBufferPrintable([]byte(tt.input))
84+
if result != tt.expected {
85+
t.Errorf("Expected %v, got %v", tt.expected, result)
86+
}
87+
})
88+
}
89+
}
90+
91+
func TestMakePrintable(t *testing.T) {
92+
var inputs = []struct {
93+
input string
94+
expected string
95+
} {
96+
{"", ""},
97+
{"hello", "hello"},
98+
{"abcdABCD0123~!@#$%^&*()_+-={}|:\"<>?,./;'[]", "abcdABCD0123~!@#$%^&*()_+-={}|:\"<>?,./;'[]"},
99+
{"Horizontal Tab and NewLine\t\t\n\n", "Horizontal Tab and NewLine\t\t\n\n"},
100+
{"(NBSP)\xa0\xa0\xa0\xa0;", "(NBSP)\xa0\xa0\xa0\xa0;"},
101+
{"\x0b(Vertical Tab)", "(Vertical Tab)"},
102+
{"\x0d(CR)", "(CR)"},
103+
{"ASCII control characters : \x00(NULL)", "ASCII control characters : (NULL)"},
104+
{"\x05(ENQ)", "(ENQ)"},
105+
{"\x0f(SI)", "(SI)"},
106+
{"\x1b(ESC)", "(ESC)"},
107+
{"\x7f(DEL)", "(DEL)"},
108+
}
109+
for _, tt := range inputs {
110+
111+
t.Run(fmt.Sprintf("Make %q printable", tt.input), func(t* testing.T){
112+
result := makePrintable(tt.input)
113+
if result != tt.expected {
114+
t.Errorf("Expected %v, got %v", tt.expected, result)
115+
}
116+
})
117+
}
118+
}
119+

0 commit comments

Comments
 (0)