Skip to content
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.

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

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

Merged
merged 4 commits into from
Mar 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions options/locale/locale_en-US.ini
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

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

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

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

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

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

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