From 69dfe81d0e9611636e9924d0eea45082a36d3081 Mon Sep 17 00:00:00 2001
From: Gusted <postmaster@gusted.xyz>
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

(cherry picked from commit e9a89a188e2cbcb264325e4880d0d4664a5b34f0)
---
 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 d121906575..a649cc9973 100644
--- a/routers/web/repo/view.go
+++ b/routers/web/repo/view.go
@@ -560,15 +560,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"}}
 		</div>
 	{{end}}
-	{{if and .HasTrailingEOLSet (not .HasTrailingEOL)}}
+	{{if .HasNoTrailingEOL}}
 		<div class="file-info-entry" data-tooltip-content="{{ctx.Locale.Tr "repo.no_eol.tooltip"}}">
 			{{ctx.Locale.Tr "repo.no_eol.text"}}
 		</div>
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)
 		})
 	})
 }