[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
This commit is contained in:
Gusted 2024-08-06 01:27:38 +02:00
parent 641b0f74b6
commit 5cf976739c
No known key found for this signature in database
GPG key ID: FD821B732837125F
5 changed files with 88 additions and 7 deletions

View file

@ -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

View file

@ -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

View file

@ -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}}

View file

@ -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"),
},
})

View file

@ -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")
})
})
}