Merge pull request '[UI] Adjust trailing EOL behavior for empty file' (#5013) from gusted/forgejo-adjust-eol into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/5013
Reviewed-by: Otto <otto@codeberg.org>

routers/web/repo/view.go

@@ -562,15 +562,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

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>

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) {
+		testEOL := func(t *testing.T, filename string, hasEOL bool) {
-			defer tests.PrintCurrentTest(t)()
+			t.Helper()
-
+			req := NewRequestf(t, "GET", "%s/src/branch/main/%s", repo.Link(), filename)
-			req := NewRequest(t, "GET", repo.Link()+"/src/branch/main/test-1")
 			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")
+		t.Run("No EOL", func(t *testing.T) {
-			resp = MakeRequest(t, req, http.StatusOK)
+			defer tests.PrintCurrentTest(t)()
-			htmlDoc = NewHTMLParser(t, resp.Body)
 
-			fileInfo = htmlDoc.Find(".file-info").Text()
+			testEOL(t, "test-1", false)
-			assert.Contains(t, fileInfo, "No EOL")
+			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")
+			testEOL(t, "test-2", true)
-			resp := MakeRequest(t, req, http.StatusOK)
+			testEOL(t, "test-4", true)
-			htmlDoc := NewHTMLParser(t, resp.Body)
+			testEOL(t, "empty", true)
-
+			testEOL(t, "seemingly-empty", true)
-			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")
 		})
 	})
 }