From 56b7478b254edc9a80c3d4e88154d977e1b2fd52 Mon Sep 17 00:00:00 2001 From: Gusted Date: Tue, 6 Aug 2024 01:27:38 +0200 Subject: [PATCH] [UI] Do not include trailing EOL character when counting lines - Adjust the counting of the number of lines of a file to match the amount of rendered lines. This simply means that a file with the content of `a\n` will be shown as having `1 line` rather than `2 lines`. This matches with the amount of lines that are being rendered (the last empty line is never rendered) and matches more with the expecation of the user (a trailing EOL is a technical detail). - In the case there's no EOL, the reason why it was counting 'incorrectly' was to show if there was a trailing EOL or not, but now text is shown to tell the user this. - Integration test added. - Resolves Codeberg/Community#1612 (cherry picked from commit 5cf976739c13bd4d490b8b80c7fd6506fe34ed8b) --- options/locale/locale_en-US.ini | 2 + routers/web/repo/view.go | 15 +++++-- templates/repo/file_info.tmpl | 5 +++ tests/integration/linguist_test.go | 6 +-- tests/integration/repo_view_test.go | 67 +++++++++++++++++++++++++++++ 5 files changed, 88 insertions(+), 7 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index c18efe8e66..929ea99bd2 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1323,6 +1323,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 diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 5295bfdb2a..d121906575 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -557,14 +557,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; - // 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. - // 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. + // 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. // 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 diff --git a/templates/repo/file_info.tmpl b/templates/repo/file_info.tmpl index 61cb9f4b8a..9cf4d28f4c 100644 --- a/templates/repo/file_info.tmpl +++ b/templates/repo/file_info.tmpl @@ -9,6 +9,11 @@ {{.NumLines}} {{ctx.Locale.TrN .NumLines "repo.line" "repo.lines"}} {{end}} + {{if and .HasTrailingEOLSet (not .HasTrailingEOL)}} +
+ {{ctx.Locale.Tr "repo.no_eol.text"}} +
+ {{end}} {{if .FileSize}}
{{ctx.Locale.TrSize .FileSize}}{{if .IsLFSFile}} ({{ctx.Locale.Tr "repo.stored_lfs"}}){{end}} diff --git a/tests/integration/linguist_test.go b/tests/integration/linguist_test.go index 332f6a8ea4..b3d7331947 100644 --- a/tests/integration/linguist_test.go +++ b/tests/integration/linguist_test.go @@ -49,7 +49,7 @@ func TestLinguistSupport(t *testing.T) { { Operation: "create", TreePath: "foo.c", - ContentReader: strings.NewReader(`#include \nint main() {\n printf("Hello world!\n");\n return 0;\n}\n`), + ContentReader: strings.NewReader("#include \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(`\n\n Hello\n\n`), + ContentReader: strings.NewReader("\n\n Hello\n\n"), }, }) diff --git a/tests/integration/repo_view_test.go b/tests/integration/repo_view_test.go index 8a77532c9b..b653d7f596 100644 --- a/tests/integration/repo_view_test.go +++ b/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") + }) + }) +}