Merge pull request '[UI] Do not include trailing EOL character when counting lines' (#4835) from gusted/forgejo-num-lines into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4835
Reviewed-by: Caesar Schinas <caesar@caesarschinas.com>
Reviewed-by: Otto <otto@codeberg.org>

options/locale/locale_en-US.ini

@@ -1331,6 +1331,8 @@ normal_view = Normal View
 line = line
 lines = lines
 from_comment = (comment)
+no_eol.text = No EOL
+no_eol.tooltip = This file doesn't contain a trailing end of line character.
 
 editor.add_file = Add file
 editor.new_file = New file

routers/web/repo/view.go

@@ -560,14 +560,21 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry) {
 			// The Open Group Base Specification: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html
 			//   empty: 0 lines; "a": 1 incomplete-line; "a\n": 1 line; "a\nb": 1 line, 1 incomplete-line;
 			// Forgejo uses the definition (like most modern editors):
-			//   empty: 0 lines; "a": 1 line; "a\n": 2 lines; "a\nb": 2 lines;
+			//   empty: 0 lines; "a": 1 line; "a\n": 1 line; "a\nb": 2 lines;
-			//   When rendering, the last empty line is not rendered in UI, while the line-number is still counted, to tell users that the file contains a trailing EOL.
+			//   When rendering, the last empty line is not rendered in U and isn't counted towards the number of lines.
-			//   To make the UI more consistent, it could use an icon mark to indicate that there is no trailing EOL, and show line-number as the rendered lines.
+			//   To tell users that the file not contains a trailing EOL, text with a tooltip is displayed in the file header.
 			// 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 {
-				ctx.Data["NumLines"] = bytes.Count(buf, []byte{'\n'}) + 1
+				numLines := bytes.Count(buf, []byte{'\n'})
+				if !hasTrailingEOL {
+					numLines++
+				}
+				ctx.Data["NumLines"] = numLines
 			}
 			ctx.Data["NumLinesSet"] = true
 

templates/repo/file_info.tmpl

@@ -9,6 +9,11 @@
 			{{.NumLines}} {{ctx.Locale.TrN .NumLines "repo.line" "repo.lines"}}
 		</div>
 	{{end}}
+	{{if and .HasTrailingEOLSet (not .HasTrailingEOL)}}
+		<div class="file-info-entry" data-tooltip-content="{{ctx.Locale.Tr "repo.no_eol.tooltip"}}">
+			{{ctx.Locale.Tr "repo.no_eol.text"}}
+		</div>
+	{{end}}
 	{{if .FileSize}}
 		<div class="file-info-entry">
 			{{ctx.Locale.TrSize .FileSize}}{{if .IsLFSFile}} ({{ctx.Locale.Tr "repo.stored_lfs"}}){{end}}

tests/integration/linguist_test.go

@@ -49,7 +49,7 @@ func TestLinguistSupport(t *testing.T) {
 					{
 						Operation:     "create",
 						TreePath:      "foo.c",
-						ContentReader: strings.NewReader(`#include <stdio.h>\nint main() {\n  printf("Hello world!\n");\n  return 0;\n}\n`),
+						ContentReader: strings.NewReader("#include <stdio.h>\nint main() {\n  printf(\"Hello world!\n\");\n  return 0;\n}\n"),
 					},
 					{
 						Operation:     "create",
@@ -64,12 +64,12 @@ func TestLinguistSupport(t *testing.T) {
 					{
 						Operation:     "create",
 						TreePath:      "cpplint.py",
-						ContentReader: strings.NewReader(`#! /usr/bin/env python\n\nprint("Hello world!")\n`),
+						ContentReader: strings.NewReader("#! /usr/bin/env python\n\nprint(\"Hello world!\")\n"),
 					},
 					{
 						Operation:     "create",
 						TreePath:      "some-file.xml",
-						ContentReader: strings.NewReader(`<?xml version="1.0"?>\n<foo>\n <bar>Hello</bar>\n</foo>\n`),
+						ContentReader: strings.NewReader("<?xml version=\"1.0\"?>\n<foo>\n <bar>Hello</bar>\n</foo>\n"),
 					},
 				})
 

tests/integration/repo_view_test.go

@@ -5,6 +5,7 @@ package integration
 
 import (
 	"fmt"
+	"net/http"
 	"net/url"
 	"strings"
 	"testing"
@@ -16,6 +17,7 @@ import (
 	"code.gitea.io/gitea/services/context"
 	"code.gitea.io/gitea/services/contexttest"
 	files_service "code.gitea.io/gitea/services/repository/files"
+	"code.gitea.io/gitea/tests"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -152,3 +154,68 @@ func TestRepoView_FindReadme(t *testing.T) {
 		})
 	})
 }
+
+func TestRepoViewFileLines(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, _ *url.URL) {
+		user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+		repo, _, f := CreateDeclarativeRepo(t, user, "file-lines", []unit_model.Type{unit_model.TypeCode}, nil, []*files_service.ChangeRepoFile{
+			{
+				Operation:     "create",
+				TreePath:      "test-1",
+				ContentReader: strings.NewReader("No newline"),
+			},
+			{
+				Operation:     "create",
+				TreePath:      "test-2",
+				ContentReader: strings.NewReader("No newline\n"),
+			},
+			{
+				Operation:     "create",
+				TreePath:      "test-3",
+				ContentReader: strings.NewReader("Two\nlines"),
+			},
+			{
+				Operation:     "create",
+				TreePath:      "test-4",
+				ContentReader: strings.NewReader("Really two\nlines\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")
+			resp := MakeRequest(t, req, http.StatusOK)
+			htmlDoc := NewHTMLParser(t, resp.Body)
+
+			fileInfo := htmlDoc.Find(".file-info").Text()
+			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)
+
+			fileInfo = htmlDoc.Find(".file-info").Text()
+			assert.Contains(t, fileInfo, "No EOL")
+		})
+
+		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")
+		})
+	})
+}