Add ui.EDITOR_EOL, default to saving files with LF again #28119

silverwind · 2023-11-19T12:29:49Z

Fixes: #28097
Replaces: #28101

Assuming we consider it a bug that people on Windows get CRLF endings without a editorconfig, we should backport this to 1.21.

silverwind · 2023-11-19T12:31:55Z

modules/setting/ui.go

@@ -34,6 +34,7 @@ var UI = struct {
 	SearchRepoDescription bool
 	OnlyShowRelevantRepos bool
 	ExploreDefaultSort    string `ini:"EXPLORE_PAGING_DEFAULT_SORT"`
+	EditorEol             string `ini:"EDITOR_EOL"`


I guess this ini tag here is unnecessary because thiscase conversation is done implicitely, but I do like this expliciteness so that the name stays greppable.

By the way, where does this case conversation happen? It's not from the ini package, right?

I think the correct Go name would even be EditorEOL

custom/conf/app.example.ini

docs/content/administration/config-cheat-sheet.en-us.md

wxiaoguang · 2023-11-21T00:54:38Z

I do not think this is right.

See the discussion #27141 (comment)

Even if you set model.setEOL(monaco.editor.EndOfLineSequence.LF);, you still get \r\n when submitting the form

I believe the best choice is to revert #27141 at the moment, make 1.21 users happy. At least, "\n" is more widely used but "\r\n" causes problems in POSIX.

silverwind · 2023-11-21T12:56:25Z

If that's because of textarea behaviour, we can fix this by setting the textarea value.

silverwind · 2023-11-21T17:19:53Z

Added the previous server-side eol convertion back, but it now respects the new setting of LF vs. CRLF too: e58dcc0

silverwind · 2023-11-21T17:29:09Z

I guess the only thing missing here is to respect editorconfig on server-side as well, but for all intends, this will restore the previous replacement to LF that was done server-side.

silverwind · 2023-11-21T17:48:39Z

I guess the only thing missing here is to respect editorconfig on server-side as well, but for all intends, this will restore the previous replacement to LF that was done server-side.

Done as well in 2dfcfe0, now the server also respects the editorconfig for this replacement. I guess it's still good to retain the client-side stuff so that copy out of monaco copies with the correct line endings.

wxiaoguang · 2023-11-22T00:19:27Z

If there is no "editorconfig", it should use the original EOL. eg: if the file was stored in "\n", the newly saved file should also use "\n"; if the file was stored in "\r\n", the newly saved file should also use "\r\n"
The config option "EDITOR_EOL" is not necessary (not used in most cases, according to the behavior above), the newly created file could just use "\n", which is widely used and doesn't cause problem in modern days.
I guess the JS trick for data-initial-value="{{JsonUtils.EncodeToString .FileContent}}" is a no-op now, it should work well by using <textarea>{{.FileContent}}</textarea> ?

delvh · 2023-12-30T13:28:21Z

modules/setting/ui.go

@@ -34,6 +34,7 @@ var UI = struct {
 	SearchRepoDescription bool
 	OnlyShowRelevantRepos bool
 	ExploreDefaultSort    string `ini:"EXPLORE_PAGING_DEFAULT_SORT"`
+	EditorEol             string `ini:"EDITOR_EOL"`


I think the correct Go name would even be EditorEOL

delvh · 2023-12-30T13:28:37Z

modules/templates/helper.go

@@ -149,6 +149,9 @@ func NewFuncMap() template.FuncMap {
 		"MermaidMaxSourceCharacters": func() int {
 			return setting.MermaidMaxSourceCharacters
 		},
+		"EditorEol": func() string {


delvh · 2023-12-30T13:28:38Z

modules/templates/helper.go

@@ -149,6 +149,9 @@ func NewFuncMap() template.FuncMap {
 		"MermaidMaxSourceCharacters": func() int {
 			return setting.MermaidMaxSourceCharacters
 		},
+		"EditorEol": func() string {


delvh · 2023-12-30T13:29:01Z

modules/util/string.go

+
+// convert all newlines in string to \n
+func ToLF(str string) string {
+	return strings.ReplaceAll(str, "\r", "")


Suggested change

return strings.ReplaceAll(str, "\r", "")

return strings.ReplaceAll(str, "\r\n", "\n")

delvh · 2023-12-30T13:31:54Z

modules/util/string.go

+
+// convert all newlines in string to \r\n
+func ToCRLF(str string) string {
+	return strings.ReplaceAll(ToLF(str), "\n", "\r\n")


Suggested change

return strings.ReplaceAll(ToLF(str), "\n", "\r\n")

return strings.ReplaceAll(str, "(?:[^\r])\n", "\r\n")

delvh · 2023-12-30T13:36:41Z

modules/util/string_test.go

@@ -45,3 +45,11 @@ func TestToSnakeCase(t *testing.T) {
 		assert.Equal(t, expected, ToSnakeCase(input))
 	}
 }
+
+func TestToLF(t *testing.T) {
+	assert.Equal(t, "\na\nbc\n\n", ToLF("\r\na\r\nb\rc\n\n"))


Suggested change

assert.Equal(t, "\na\nbc\n\n", ToLF("\r\na\r\nb\rc\n\n"))

assert.Equal(t, "\r\na\nbc\n\n", ToLF("\r\r\na\r\nb\rc\n\n"))

Or how do we want to handle this case?
I can live both with the current approach of removing it completely, and my proposed approach of only converting \r\n -> \n

wxiaoguang · 2024-01-26T14:10:48Z

If there is no "editorconfig", it should use the original EOL. eg: if the file was stored in "\n", the newly saved file should also use "\n"; if the file was stored in "\r\n", the newly saved file should also use "\r\n"

The same problem as BOM: #28935 (comment)

if the file was stored in "\n", the newly saved file should also use "\n"; if the file was stored in "\r\n", the newly saved file should also use "\r\n"
(and more) if the file was stored with some BOM, the newly saved file should also use that BOM

silverwind · 2024-01-27T00:54:01Z

if the file was stored in "\n", the newly saved file should also use "\n"; if the file was stored in "\r\n", the newly saved file should also use "\r\n"

Yes, like with BOM we should just round-trip the file unmodified. The only problem is new files and files with equal amount of CRLF and LF, for those we need to bias towards one format, which could come from editorconfig with fallback to global server config.

I think the approach here is not correct and we should actually avoid backend convertion completely. So I think it's better to make a new PR, maybe I will have a look later, but feel free to raise something @wxiaoguang.

wxiaoguang · 2024-01-27T09:14:27Z

I think the approach here is not correct and we should actually avoid backend convertion completely.

IMO at the moment, backend convertion can't be avoided. Because HTML textarea always passes "\r\n" to backend, it is the standard.

silverwind · 2024-01-29T17:19:27Z

I think the approach here is not correct and we should actually avoid backend convertion completely.

IMO at the moment, backend convertion can't be avoided. Because HTML textarea always passes "\r\n" to backend, it is the standard.

Yeah, but we can at least avoid altering the content until the point where it reaches the editor. Maybe it's ideal to use a data- attribute to ensure the browser does no shenenigans with the textarea value even when reading it.

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 19, 2023

pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 19, 2023

silverwind added the backport/v1.21 This PR should be backported to Gitea 1.21 label Nov 19, 2023

github-actions bot added modifies/frontend modifies/docs labels Nov 19, 2023

silverwind added the type/enhancement An improvement of existing functionality label Nov 19, 2023

silverwind mentioned this pull request Nov 19, 2023

Revert "Fix EOL handling in web editor" #28101

Merged

silverwind commented Nov 19, 2023

View reviewed changes

custom/conf/app.example.ini Outdated Show resolved Hide resolved

silverwind commented Nov 19, 2023

View reviewed changes

docs/content/administration/config-cheat-sheet.en-us.md Outdated Show resolved Hide resolved

denyskon approved these changes Nov 19, 2023

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 19, 2023

pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 21, 2023

silverwind added 6 commits November 21, 2023 18:47

Add ui.EDITOR_EOL setting and use ist instead of monaco's detection

04bb2a5

Update custom/conf/app.example.ini

d39adbb

Update docs/content/administration/config-cheat-sheet.en-us.md

ab1e9ca

add server-side convertion

586b8d3

fix name

c071ad5

respect editorconfig server-side too

2dfcfe0

github-actions bot added modifies/translation modifies/internal labels Nov 21, 2023

silverwind force-pushed the editoreol branch from 8806cc5 to 2dfcfe0 Compare November 21, 2023 17:47

pull-request-size bot removed the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 21, 2023

github-actions bot removed the modifies/translation label Nov 21, 2023

github-actions bot removed the modifies/internal label Nov 21, 2023

fix lint and refactor

2fdf6d9

pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 21, 2023

silverwind added 2 commits November 21, 2023 23:44

Merge branch 'main' into editoreol

e02ccd7

reword docs

6aaee67

silverwind changed the title ~~Add ui.EDITOR_EOL setting and use ist instead of monaco's detection~~ Add ui.EDITOR_EOL setting, default to saving files with LF again Nov 21, 2023

silverwind changed the title ~~Add ui.EDITOR_EOL setting, default to saving files with LF again~~ Add ui.EDITOR_EOL, default to saving files with LF again Nov 21, 2023

pass setting via data attribute

c321a68

delvh reviewed Dec 30, 2023

View reviewed changes

wxiaoguang mentioned this pull request Jan 26, 2024

Preserve BOM in web editor #28935

Merged

silverwind closed this Jan 27, 2024

silverwind deleted the editoreol branch January 29, 2024 17:19

wxiaoguang mentioned this pull request Jan 30, 2024

Wiki pages are stored as/converted to CRLF line endings #17541

Open

github-actions bot locked as resolved and limited conversation to collaborators Mar 4, 2024

lunny removed the backport/v1.21 This PR should be backported to Gitea 1.21 label Mar 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ui.EDITOR_EOL, default to saving files with LF again #28119

Add ui.EDITOR_EOL, default to saving files with LF again #28119

silverwind commented Nov 19, 2023 •

edited

Loading

silverwind Nov 19, 2023 •

edited

Loading

delvh Dec 30, 2023

wxiaoguang commented Nov 21, 2023

silverwind commented Nov 21, 2023

silverwind commented Nov 21, 2023 •

edited

Loading

silverwind commented Nov 21, 2023

silverwind commented Nov 21, 2023 •

edited

Loading

wxiaoguang commented Nov 22, 2023

delvh Dec 30, 2023

delvh Dec 30, 2023

delvh Dec 30, 2023

delvh Dec 30, 2023

delvh Dec 30, 2023

delvh Dec 30, 2023

delvh Dec 30, 2023

wxiaoguang commented Jan 26, 2024

silverwind commented Jan 27, 2024 •

edited

Loading

wxiaoguang commented Jan 27, 2024

silverwind commented Jan 29, 2024 •

edited

Loading

	return strings.ReplaceAll(str, "\r", "")
	return strings.ReplaceAll(str, "\r\n", "\n")

	return strings.ReplaceAll(ToLF(str), "\n", "\r\n")
	return strings.ReplaceAll(str, "(?:[^\r])\n", "\r\n")

	assert.Equal(t, "\na\nbc\n\n", ToLF("\r\na\r\nb\rc\n\n"))
	assert.Equal(t, "\r\na\nbc\n\n", ToLF("\r\r\na\r\nb\rc\n\n"))

Add ui.EDITOR_EOL, default to saving files with LF again #28119

Add ui.EDITOR_EOL, default to saving files with LF again #28119

Conversation

silverwind commented Nov 19, 2023 • edited Loading

silverwind Nov 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wxiaoguang commented Nov 21, 2023

silverwind commented Nov 21, 2023

silverwind commented Nov 21, 2023 • edited Loading

silverwind commented Nov 21, 2023

silverwind commented Nov 21, 2023 • edited Loading

wxiaoguang commented Nov 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wxiaoguang commented Jan 26, 2024

silverwind commented Jan 27, 2024 • edited Loading

wxiaoguang commented Jan 27, 2024

silverwind commented Jan 29, 2024 • edited Loading

silverwind commented Nov 19, 2023 •

edited

Loading

silverwind Nov 19, 2023 •

edited

Loading

silverwind commented Nov 21, 2023 •

edited

Loading

silverwind commented Nov 21, 2023 •

edited

Loading

silverwind commented Jan 27, 2024 •

edited

Loading

silverwind commented Jan 29, 2024 •

edited

Loading