Editor error message misleading due to re-used key. #29859

buckybytes · 2024-03-17T05:53:30Z

The error message:

editor.file_changed_while_editing = The file contents have changed since you started editing. <a target="_blank" rel="noopener noreferrer" href="%s">Click here</a> to see them or Commit Changes again to overwrite them.

Is re-used in inappropriate contexts. The link in the key goes to a 404 when the key is used in a situation where the file contents have not changed.

Added two new keys to differentiate commit id mismatch and push out of date conditions.

silverwind · 2024-03-17T22:14:51Z

options/locale/locale_en-US.ini

@@ -1312,6 +1312,8 @@ editor.file_editing_no_longer_exists = The file being edited, "%s", no longer ex
 editor.file_deleting_no_longer_exists = The file being deleted, "%s", no longer exists in this repository.
 editor.file_changed_while_editing = The file contents have changed since you started editing. <a target="_blank" rel="noopener noreferrer" href="%s">Click here</a> to see them or <strong>Commit Changes again</strong> to overwrite them.
 editor.file_already_exists = A file named "%s" already exists in this repository.
+editor.commit_id_not_matching = The Commit ID does not match the ID when you began editing.  Commit into a patch branch and then merge.


Wouldn't "Commit hash" be a more accurate name in git terminology?

I wanted the message to match the key.

Hmm I see the term "Commit ID" is deeply rooted in the backend, and some other translations also use it, but with inconsistent casing:

pulls.wrong_commit_id = "commit id must be a commit id on the target branch" pulls.merge_commit_id = The merge commit ID

Yeah, there's a mess all over as far as consistency goes in the locale file. I just wanted to get to the bottom of which case was being triggered when I got the error, because mine was definitely not the file changing.

The error message: `editor.file_changed_while_editing = The file contents have changed since you started editing. <a target="_blank" rel="noopener noreferrer" href="%s">Click here</a> to see them or Commit Changes again to overwrite them.` Is re-used in inappropriate contexts. The link in the key goes to a 404 when the key is used in a situation where the file contents have not changed. Added two new keys to differentiate commit id mismatch and push out of date conditions.

@buckybytes

Backport #29859 by @buckybytes The error message: `editor.file_changed_while_editing = The file contents have changed since you started editing. <a target="_blank" rel="noopener noreferrer" href="%s">Click here</a> to see them or Commit Changes again to overwrite them.` Is re-used in inappropriate contexts. The link in the key goes to a 404 when the key is used in a situation where the file contents have not changed. Added two new keys to differentiate commit id mismatch and push out of date conditions. Co-authored-by: buckybytes <158571971+buckybytes@users.noreply.github.com>

wxiaoguang · 2024-03-18T09:06:12Z

routers/web/repo/editor.go

@@ -333,9 +333,9 @@ func editFilePost(ctx *context.Context, form forms.EditRepoFileForm, isNewFile b
 				ctx.Error(http.StatusInternalServerError, err.Error())
 			}
 		} else if models.IsErrCommitIDDoesNotMatch(err) {
-			ctx.RenderWithErr(ctx.Tr("repo.editor.file_changed_while_editing", ctx.Repo.RepoLink+"/compare/"+util.PathEscapeSegments(form.LastCommit)+"..."+util.PathEscapeSegments(ctx.Repo.CommitID)), tplEditFile, &form)
+			ctx.RenderWithErr(ctx.Tr("repo.editor.commit_id_not_matching", ctx.Repo.RepoLink+"/compare/"+util.PathEscapeSegments(form.LastCommit)+"..."+util.PathEscapeSegments(ctx.Repo.CommitID)), tplEditFile, &form)


Is this code right? I don't see %s in commit_id_not_matching, but this Tr has extra arguments?

wxiaoguang · 2024-03-18T09:06:35Z

routers/web/repo/editor.go

 		} else if git.IsErrPushOutOfDate(err) {
-			ctx.RenderWithErr(ctx.Tr("repo.editor.file_changed_while_editing", ctx.Repo.RepoLink+"/compare/"+util.PathEscapeSegments(form.LastCommit)+"..."+util.PathEscapeSegments(form.NewBranchName)), tplEditFile, &form)
+			ctx.RenderWithErr(ctx.Tr("repo.editor.push_out_of_date", ctx.Repo.RepoLink+"/compare/"+util.PathEscapeSegments(form.LastCommit)+"..."+util.PathEscapeSegments(form.NewBranchName)), tplEditFile, &form)


As above, is this code right? I don't see %s in push_out_of_date, but this Tr has extra arguments?

* giteaofficial/main: Fix missing error check of bufio.Scanner (go-gitea#29882) Remove unused error in graceful manager (go-gitea#29871) Migrate border and margin classes to Tailwind (go-gitea#29828) Only do counting when count_only=true for repo dashboard (go-gitea#29884) Editor error message misleading due to re-used key. (go-gitea#29859) [skip ci] Updated licenses and gitignores move some scripts from 'build' to 'tools' directory, misc refactors (go-gitea#29844) Fix missing code in the user profile (go-gitea#29865) Upgrade Go 1.22 and upgrade dependency (go-gitea#29869) Fix the wrong locale key of searching users (go-gitea#29868) fix telegram webhook (go-gitea#29864) Fix user id column case (go-gitea#29863) Avoid JS error on issue/pr list when logged out (go-gitea#29854) Refactor clone-panel styles (go-gitea#29861) Simplify README (go-gitea#29827) Load citation JS only when needed (go-gitea#29855) Fix semantic.json (go-gitea#29860) # Conflicts: # templates/repo/wiki/revision.tmpl # templates/repo/wiki/view.tmpl

wxiaoguang · 2024-03-22T04:36:32Z

-> Fix some pending problems #29985

These changes are quite independent and trivial, so I don't want to open too many PRs. * #29882 (comment) * the `f.Close` should be called properly * the error message could be more meaningful (#29882 (review)) * #29859 (review) * the new translation strings don't take arguments * #28710 (comment) * stale for long time * #28140 * a form was forgotten to be changed to work with backend code

These changes are quite independent and trivial, so I don't want to open too many PRs. * go-gitea/gitea#29882 (comment) * the `f.Close` should be called properly * the error message could be more meaningful (go-gitea/gitea#29882 (review)) * go-gitea/gitea#29859 (review) * the new translation strings don't take arguments * go-gitea/gitea#28710 (comment) * stale for long time * #28140 * a form was forgotten to be changed to work with backend code (cherry picked from commit 226231ea27d4f2b0f09fa4efb39501507613b284) Conflicts: templates/repo/issue/view_content/pull.tmpl discarded because unexplained templates/status/404.tmpl implemented differently in Forgejo

buckybytes added 2 commits March 17, 2024 00:45

Update editor.go

dd96278

Update locale_en-US.ini

d4368ef

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 17, 2024

pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 17, 2024

github-actions bot added modifies/translation modifies/go Pull requests that update Go code labels Mar 17, 2024

Merge branch 'main' into editorerror

f2bfc4b

lunny approved these changes Mar 17, 2024

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 Mar 17, 2024

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

silverwind reviewed Mar 17, 2024

View reviewed changes

silverwind approved these changes Mar 17, 2024

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 17, 2024

lunny enabled auto-merge (squash) March 18, 2024 01:57

lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 18, 2024

Merge branch 'main' into editorerror

f1b8dd4

lunny merged commit 16e3600 into go-gitea:main Mar 18, 2024
26 checks passed

GiteaBot added this to the 1.23.0 milestone Mar 18, 2024

GiteaBot mentioned this pull request Mar 18, 2024

Editor error message misleading due to re-used key. (#29859) #29876

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 Mar 18, 2024

wxiaoguang reviewed Mar 18, 2024

View reviewed changes

wxiaoguang mentioned this pull request Mar 22, 2024

Fix some pending problems #29985

Merged

lunny modified the milestones: 1.23.0, 1.22.0 Mar 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Editor error message misleading due to re-used key. #29859

Editor error message misleading due to re-used key. #29859

buckybytes commented Mar 17, 2024

silverwind Mar 17, 2024

buckybytes Mar 17, 2024

silverwind Mar 17, 2024 •

edited

buckybytes Mar 18, 2024

wxiaoguang Mar 18, 2024

wxiaoguang Mar 18, 2024

wxiaoguang commented Mar 22, 2024

Editor error message misleading due to re-used key. #29859

Editor error message misleading due to re-used key. #29859

Conversation

buckybytes commented Mar 17, 2024

silverwind Mar 17, 2024

Choose a reason for hiding this comment

buckybytes Mar 17, 2024

Choose a reason for hiding this comment

silverwind Mar 17, 2024 • edited

Choose a reason for hiding this comment

buckybytes Mar 18, 2024

Choose a reason for hiding this comment

wxiaoguang Mar 18, 2024

Choose a reason for hiding this comment

wxiaoguang Mar 18, 2024

Choose a reason for hiding this comment

wxiaoguang commented Mar 22, 2024

silverwind Mar 17, 2024 •

edited