Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lunny merged 4 commits into from
Mar 18, 2024
Merged

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dd96278
Update editor.go
buckybytes Mar 17, 2024
d4368ef
Update locale_en-US.ini
buckybytes Mar 17, 2024
f2bfc4b
Merge branch 'main' into editorerror
buckybytes Mar 17, 2024
f1b8dd4
Merge branch 'main' into editorerror
GiteaBot Mar 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions options/locale/locale_en-US.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Member

@silverwind silverwind Mar 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@buckybytes buckybytes Mar 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted the message to match the key.

Copy link
Member

@silverwind silverwind Mar 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@buckybytes buckybytes Mar 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

editor.push_out_of_date = The push appears to be out of date.
editor.commit_empty_file_header = Commit an empty file
editor.commit_empty_file_text = The file you're about to commit is empty. Proceed?
editor.no_changes_to_show = There are no changes to show.
Expand Down
4 changes: 2 additions & 2 deletions routers/web/repo/editor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

@wxiaoguang wxiaoguang Mar 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

} 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)
Copy link
Contributor

@wxiaoguang wxiaoguang Mar 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

} else if git.IsErrPushRejected(err) {
errPushRej := err.(*git.ErrPushRejected)
if len(errPushRej.Message) == 0 {
Expand Down