From e9a89a188e2cbcb264325e4880d0d4664a5b34f0 Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 19 Aug 2024 17:47:59 +0200 Subject: [PATCH] [UI] Adjust trailing EOL behavior for empty file - Follow up #4835 - Currently for empty files (file size is shown in the file header) the "No EOL" information is being shown, even though it doesn't really make sense to show that for empty files. - Add integration test. - Ref: https://codeberg.org/Codeberg/Community/issues/1612#issuecomment-2169437 --- routers/web/repo/view.go | 9 +++--- templates/repo/file_info.tmpl | 2 +- tests/integration/repo_view_test.go | 50 ++++++++++++++++------------- 3 files changed, 33 insertions(+), 28 deletions(-) diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index e966917640..05f6eadc0c 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -564,15 +564,16 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry) { // empty: 0 lines; "a": 1 line; "a\n": 1 line; "a\nb": 2 lines; // When rendering, the last empty line is not rendered in U and isn't counted towards the number of lines. // To tell users that the file not contains a trailing EOL, text with a tooltip is displayed in the file header. + // Trailing EOL is only considered if the file has content. // This NumLines is only used for the display on the UI: "xxx lines" - hasTrailingEOL := bytes.HasSuffix(buf, []byte{'\n'}) - ctx.Data["HasTrailingEOL"] = hasTrailingEOL - ctx.Data["HasTrailingEOLSet"] = true if len(buf) == 0 { ctx.Data["NumLines"] = 0 } else { + hasNoTrailingEOL := !bytes.HasSuffix(buf, []byte{'\n'}) + ctx.Data["HasNoTrailingEOL"] = hasNoTrailingEOL + numLines := bytes.Count(buf, []byte{'\n'}) - if !hasTrailingEOL { + if hasNoTrailingEOL { numLines++ } ctx.Data["NumLines"] = numLines diff --git a/templates/repo/file_info.tmpl b/templates/repo/file_info.tmpl index 9cf4d28f4c..6ae7c15a26 100644 --- a/templates/repo/file_info.tmpl +++ b/templates/repo/file_info.tmpl @@ -9,7 +9,7 @@ {{.NumLines}} {{ctx.Locale.TrN .NumLines "repo.line" "repo.lines"}} {{end}} - {{if and .HasTrailingEOLSet (not .HasTrailingEOL)}} + {{if .HasNoTrailingEOL}}
{{ctx.Locale.Tr "repo.no_eol.text"}}
diff --git a/tests/integration/repo_view_test.go b/tests/integration/repo_view_test.go index b653d7f596..cc4084e21c 100644 --- a/tests/integration/repo_view_test.go +++ b/tests/integration/repo_view_test.go @@ -179,43 +179,47 @@ func TestRepoViewFileLines(t *testing.T) { TreePath: "test-4", ContentReader: strings.NewReader("Really two\nlines\n"), }, + { + Operation: "create", + TreePath: "empty", + ContentReader: strings.NewReader(""), + }, + { + Operation: "create", + TreePath: "seemingly-empty", + ContentReader: strings.NewReader("\n"), + }, }) defer f() - t.Run("No EOL", func(t *testing.T) { - defer tests.PrintCurrentTest(t)() - - req := NewRequest(t, "GET", repo.Link()+"/src/branch/main/test-1") + testEOL := func(t *testing.T, filename string, hasEOL bool) { + t.Helper() + req := NewRequestf(t, "GET", "%s/src/branch/main/%s", repo.Link(), filename) resp := MakeRequest(t, req, http.StatusOK) htmlDoc := NewHTMLParser(t, resp.Body) fileInfo := htmlDoc.Find(".file-info").Text() - assert.Contains(t, fileInfo, "No EOL") + if hasEOL { + assert.NotContains(t, fileInfo, "No EOL") + } else { + assert.Contains(t, fileInfo, "No EOL") + } + } - req = NewRequest(t, "GET", repo.Link()+"/src/branch/main/test-3") - resp = MakeRequest(t, req, http.StatusOK) - htmlDoc = NewHTMLParser(t, resp.Body) + t.Run("No EOL", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() - fileInfo = htmlDoc.Find(".file-info").Text() - assert.Contains(t, fileInfo, "No EOL") + testEOL(t, "test-1", false) + testEOL(t, "test-3", false) }) t.Run("With EOL", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - req := NewRequest(t, "GET", repo.Link()+"/src/branch/main/test-2") - resp := MakeRequest(t, req, http.StatusOK) - htmlDoc := NewHTMLParser(t, resp.Body) - - fileInfo := htmlDoc.Find(".file-info").Text() - assert.NotContains(t, fileInfo, "No EOL") - - req = NewRequest(t, "GET", repo.Link()+"/src/branch/main/test-4") - resp = MakeRequest(t, req, http.StatusOK) - htmlDoc = NewHTMLParser(t, resp.Body) - - fileInfo = htmlDoc.Find(".file-info").Text() - assert.NotContains(t, fileInfo, "No EOL") + testEOL(t, "test-2", true) + testEOL(t, "test-4", true) + testEOL(t, "empty", true) + testEOL(t, "seemingly-empty", true) }) }) }