Fix EOL handling in web editor #27141

silverwind · 2023-09-19T21:08:28Z

Fixes #27136.

This does the following for Monaco's EOL setting:

Use editorconfig setting if present
Use the file's dominant line ending as detected by monaco, which uses LF for empty file

silverwind · 2023-09-19T22:12:13Z

At least I can rule out golang from stripping the \r: https://go.dev/play/p/ZY2wW6BKXyV

templates/repo/editor/edit.tmpl

wxiaoguang · 2023-09-20T00:33:22Z

web_src/js/features/codeeditor.js

@@ -105,14 +105,26 @@ export async function createMonaco(textarea, filename, editorOpts) {
  monaco.languages.register({id: 'vs.editor.nullLanguage'});
  monaco.languages.setLanguageConfiguration('vs.editor.nullLanguage', {});

+  // TODO: there must be a better way to preserve CRLF in the template rendering


I guess there is no other way besides using JS.

HTML standard does play tricks with CRLF/LF for textarea value.

I think it's a self-inflicted issue by gitea's HTML rendering. But no idea where the removal of \r happens.

It is not related to Gitea.

Respect original content when creating secrets #24745 (comment)

For historical reasons, the element's value is normalized in three different ways for three different purposes.
The raw value is the value as it was originally set. It is not normalized.
The API value ... It is normalized so that line breaks use U+000A LINE FEED (LF) characters.

textarea.value is the API value IIRC.

I also tried passing as data-value and the \r was stripped as well there. So it's not related to textarea.value. It must be something in our HTML renderer.

I do not see so. I think everything just works well.

The standard has clarified that there is no CR but only LF when parsing HTML. I do not know how to explain more.

Nothing is unexpected, everything is right from HTML/Golang/GoTemplate/Gitea side.

The only wrong thing is that you shouldn't emit "\r" into HTML directly if you want to keep it.

2. I fail to see how that is related to the problem. I have shown that regular go template does not have the problem, but ours rendering does, so it's a bug in our rendering.

Your code is wrong. I have already shown that our code renders "\r\n" correctly: #27141 (comment)

Ah, you mean the browser discards it during HTML parsing stage? I will check with curl to confirm. If this is true, then yes, we need to encode it.

I'm also wondering which encoding would be best in terms of output size and parsing speed. Maybe JSON-encoding the string might be best.

curl confirms, it's output correctly. Damn browsers and their silly parsers:

curl -s http://localhost:3500/devtest/gitea-ui | grep data-foo= | xxd 00000000: 3c64 6976 2063 6c61 7373 3d22 7061 6765 <div class="page 00000010: 2d63 6f6e 7465 6e74 2064 6576 7465 7374 -content devtest 00000020: 2075 6920 636f 6e74 6169 6e65 7222 2064 ui container" d 00000030: 6174 612d 666f 6f3d 2261 0d0a ata-foo="a..

Fixed with f394554.

silverwind · 2023-09-21T09:55:36Z

It's ready now. Data is passed through JSON encoding from backend to frontend now.

wxiaoguang

PR description also needs to update.

silverwind · 2023-09-21T10:16:12Z

Removed the WIP note in description.

Fixes go-gitea#27136. This does the following for Monaco's EOL setting: 1. Use editorconfig setting if present 2. Use the file's dominant line ending as detected by monaco, which uses LF for empty file

@silverwind

Backport #27141 by @silverwind Fixes #27136. This does the following for Monaco's EOL setting: 1. Use editorconfig setting if present 2. Use the file's dominant line ending as detected by monaco, which uses LF for empty file Co-authored-by: silverwind <me@silverwind.io>

* giteaofficial/main: Add missing public user visibility in user details page (go-gitea#27246) Use mask-based fade-out effect for `.new-menu` (go-gitea#27181) [skip ci] Updated translations via Crowdin Fix z-index on markdown completion (go-gitea#27237) Update database-preparation and add note re: MariaDB (go-gitea#27232) cleanup locale function usage (go-gitea#27227) Fix EOL handling in web editor (go-gitea#27141) Fix PushEvent NullPointerException jenkinsci/github-plugin (go-gitea#27203) fix issues on action runners page (go-gitea#27226) Fix Fomantic UI dropdown icon bug when there is a search input in menu (go-gitea#27225) Update go-enry to 2.8.5 (go-gitea#27215) Update nodejs installation method in release container (go-gitea#27207) Quote table `release` in sql queries (go-gitea#27205) Fix push mirror, wrong timestamp format (go-gitea#27153) Allow copying issue comment link on archived repos and when not logged in (go-gitea#27193) fix: text decorator on issue sidebar menu label (go-gitea#27206) Update JS and Poetry dependencies and eslint (go-gitea#27200) Remove some dead code (go-gitea#27196) # Conflicts: # templates/repo/issue/view_content/context_menu.tmpl

testdasi · 2023-11-17T02:20:26Z

The fixes have broken it. All my files are LF and when edit in browser, it always changes to Windows. This is very frustrating. Were the fixes regression--tested at all?

I can't possibly be the only person editing files in a Windows browser. Both Firefox and Chrome are affected.

wxiaoguang · 2023-11-17T02:25:16Z

If there is no quick fix, let's revert this PR.

This reverts commit 3a187ea.

lunny · 2023-11-17T03:33:04Z

If there is no quick fix, let's revert this PR.

I have also encounter this problem.

delvh · 2023-11-17T12:08:32Z

Were the fixes regression--tested at all?

By the way, as you've noticed yourself now, it isn't quite easy to catch a Windows bug when most maintainers are using Linux/Mac… 😉

Reverts #27141 close #28097

Reverts go-gitea#27141 close go-gitea#28097

@lng2020

Backport #28101 by @lng2020 Reverts #27141 close #28097 Co-authored-by: Nanguan Lin <70063547+lng2020@users.noreply.github.com>

kuon · 2023-11-22T23:31:10Z

Yes I just encountered the bug where all LF got converted to CRLF, with v 1.21.0. The repository has no .gitattribute.

testdasi · 2023-11-23T00:02:14Z

Yes I just encountered the bug where all LF got converted to CRLF, with v 1.21.0. The repository has no .gitattribute.

You might want to try 1.21.0 nightly for the time being. I'm using nightly and the change was reverted there.

I still can't understand the rationale behind using a user's OS to force CRLF EOL even when the editor correct detected non-Windows files.

wxiaoguang · 2023-11-23T00:08:26Z

I still can't understand the rationale behind using a user's OS to force CRLF EOL even when the editor correct detected non-Windows files.

It's not related to OS, but it is a changed behavior caused by backend code for how to use HTML textarea content.

Fix EOL handling in web editor

db3d572

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

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

silverwind marked this pull request as draft September 19, 2023 21:08

github-actions bot added the topic/ui Change the appearance of the Gitea UI label Sep 19, 2023

use monaco's eol detection

5566a2d

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

silverwind added 2 commits September 20, 2023 00:29

comment tweak

598a2e6

refactor

c585eaa

silverwind commented Sep 19, 2023

View reviewed changes

templates/repo/editor/edit.tmpl Outdated Show resolved Hide resolved

Update templates/repo/editor/edit.tmpl

1626c14

wxiaoguang reviewed Sep 20, 2023

View reviewed changes

templates/repo/editor/edit.tmpl Outdated Show resolved Hide resolved

wxiaoguang reviewed Sep 20, 2023

View reviewed changes

silverwind added 3 commits September 20, 2023 21:13

illustrate CR issue

f93a1b2

add devtest showcase

d6fd3f6

encode with json, remove debugs

f394554

silverwind marked this pull request as ready for review September 21, 2023 09:55

Merge branch 'main' into eol

36b68c5

silverwind added backport/v1.21 This PR should be backported to Gitea 1.21 type/bug labels Sep 21, 2023

silverwind added 3 commits September 21, 2023 11:57

fix comment

60369e0

update comment

4557329

better link

8545d15

wxiaoguang approved these changes Sep 21, 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 Sep 21, 2023

GiteaBot added 2 commits September 25, 2023 02:12

Merge branch 'main' into eol

6bfd711

Merge branch 'main' into eol

e75a89f

silverwind merged commit 3a187ea into go-gitea:main Sep 24, 2023
25 checks passed

GiteaBot added this to the 1.22.0 milestone Sep 24, 2023

GiteaBot mentioned this pull request Sep 24, 2023

Fix EOL handling in web editor (#27141) #27234

Merged

GiteaBot added backport/done All backports for this PR have been created and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Sep 24, 2023

silverwind deleted the eol branch September 24, 2023 20:30

lng2020 mentioned this pull request Sep 26, 2023

When creating the initial README.md from the project creation page, the text lines have CRLF line endings when the client is Windows, but Gitea is running in Linux #27120

Open

wxiaoguang mentioned this pull request Oct 30, 2023

Web GUI commit diff shows large change for a single line change #27837

Closed

lng2020 added a commit to lng2020/gitea that referenced this pull request Nov 17, 2023

Revert "Fix EOL handling in web editor (go-gitea#27141)"

39bde96

This reverts commit 3a187ea.

lng2020 mentioned this pull request Nov 17, 2023

Revert "Fix EOL handling in web editor" #28101

Merged

wxiaoguang mentioned this pull request Nov 21, 2023

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

Closed

lunny pushed a commit that referenced this pull request Nov 22, 2023

Revert "Fix EOL handling in web editor" (#28101)

37ed92d

Reverts #27141 close #28097

GiteaBot mentioned this pull request Nov 22, 2023

Revert "Fix EOL handling in web editor" (#28101) #28172

Merged

GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Nov 22, 2023

Revert "Fix EOL handling in web editor" (go-gitea#28101)

62a5d3e

Reverts go-gitea#27141 close go-gitea#28097

silverwind pushed a commit that referenced this pull request Nov 22, 2023

Revert "Fix EOL handling in web editor" (#28101) (#28172)

a2314ca

Backport #28101 by @lng2020 Reverts #27141 close #28097 Co-authored-by: Nanguan Lin <70063547+lng2020@users.noreply.github.com>

wxiaoguang mentioned this pull request Dec 4, 2023

GitHooks UI (Web IDE) is broken on update to Gitea 1.21.0 #28344

Closed

go-gitea locked as resolved and limited conversation to collaborators Dec 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix EOL handling in web editor #27141

Fix EOL handling in web editor #27141

silverwind commented Sep 19, 2023 •

edited

Loading

silverwind commented Sep 19, 2023

wxiaoguang Sep 20, 2023

silverwind Sep 20, 2023 •

edited

Loading

wxiaoguang Sep 20, 2023 •

edited

Loading

silverwind Sep 20, 2023 •

edited

Loading

wxiaoguang Sep 20, 2023

wxiaoguang Sep 21, 2023

wxiaoguang Sep 21, 2023

silverwind Sep 21, 2023 •

edited

Loading

silverwind Sep 21, 2023

silverwind Sep 21, 2023

silverwind commented Sep 21, 2023

wxiaoguang left a comment

silverwind commented Sep 21, 2023

testdasi commented Nov 17, 2023

wxiaoguang commented Nov 17, 2023

lunny commented Nov 17, 2023

delvh commented Nov 17, 2023

kuon commented Nov 22, 2023

testdasi commented Nov 23, 2023

wxiaoguang commented Nov 23, 2023

Fix EOL handling in web editor #27141

Fix EOL handling in web editor #27141

Conversation

silverwind commented Sep 19, 2023 • edited Loading

silverwind commented Sep 19, 2023

wxiaoguang Sep 20, 2023

Choose a reason for hiding this comment

silverwind Sep 20, 2023 • edited Loading

Choose a reason for hiding this comment

wxiaoguang Sep 20, 2023 • edited Loading

Choose a reason for hiding this comment

silverwind Sep 20, 2023 • edited Loading

Choose a reason for hiding this comment

wxiaoguang Sep 20, 2023

Choose a reason for hiding this comment

wxiaoguang Sep 21, 2023

Choose a reason for hiding this comment

wxiaoguang Sep 21, 2023

Choose a reason for hiding this comment

silverwind Sep 21, 2023 • edited Loading

Choose a reason for hiding this comment

silverwind Sep 21, 2023

Choose a reason for hiding this comment

silverwind Sep 21, 2023

Choose a reason for hiding this comment

silverwind commented Sep 21, 2023

wxiaoguang left a comment

Choose a reason for hiding this comment

silverwind commented Sep 21, 2023

testdasi commented Nov 17, 2023

wxiaoguang commented Nov 17, 2023

lunny commented Nov 17, 2023

delvh commented Nov 17, 2023

kuon commented Nov 22, 2023

testdasi commented Nov 23, 2023

wxiaoguang commented Nov 23, 2023

silverwind commented Sep 19, 2023 •

edited

Loading

silverwind Sep 20, 2023 •

edited

Loading

wxiaoguang Sep 20, 2023 •

edited

Loading

silverwind Sep 20, 2023 •

edited

Loading

silverwind Sep 21, 2023 •

edited

Loading