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

Localize the issue due date events #24055

Closed
wants to merge 1 commit into from

Conversation

yardenshoham
Copy link
Member

@yardenshoham
Copy link
Member Author

yardenshoham commented Apr 11, 2023

I really don't like the way I had to mess with the dictionary files, is there any way to avoid this?

I tried

@@ -295,7 +295,7 @@
                                {{template "shared/user/avatarlink" dict "Context" $.Context "user" .Poster}}
                                <span class="text grey muted-links">
                                        {{template "shared/user/authorlink" .Poster}}
-                                       {{$.locale.Tr "repo.issues.due_date_added" .Content $createdStr | Safe}}
+                                       {{$.locale.Tr "repo.issues.due_date_added" (template "shared/datetime/short" (dict "Datetime" .Content "Fallback" .Content)) $createdStr | Safe}}
                                </span>

but apparently that's illegal...

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 11, 2023
Comment on lines +298 to +300
{{$.locale.Tr "repo.issues.due_date_added"}}
{{template "shared/datetime/short" (dict "Datetime" .Content "Fallback" .Content)}}
{{$createdStr}}
Copy link
Member

Choose a reason for hiding this comment

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

I can only see one suitable solution that will take some refactoring:
Add a method Comment.AsTimeStamp() that only works on comments with the correct comment types where the content of the comment is a timestamp in string form.
Then, you can call .AsTimeStamp().TimeSinceUnix().
You may even decide to panic in this new method instead of throwing an error as it can only happen when someone calls this method in the wrong place.

Copy link
Contributor

@wxiaoguang wxiaoguang Apr 11, 2023

Choose a reason for hiding this comment

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

It could be like this:

{{$s := printf "<relative-time ...>" ....}}

Then pass the $s to Tr

Comment on lines +1508 to +1511
issues.due_date_added = "added the due date"
issues.due_date_modified_from = "modified the due date from"
issues.due_date_modified_to = "to"
issues.due_date_remove = "removed the due date"
Copy link
Contributor

Choose a reason for hiding this comment

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

It's better not to do so. At least, wait for a proper i18n system (with correct datetime support)

@silverwind silverwind added the type/enhancement An improvement of existing functionality label Apr 12, 2023
@yardenshoham yardenshoham deleted the due-date-time branch April 14, 2023 09:51
@go-gitea go-gitea locked and limited conversation to collaborators Apr 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants