-
Notifications
You must be signed in to change notification settings - Fork 150
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
Base setup for i18n for server with go-i18n library #782
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
{ | ||
"git.luolix.topmand.help.title": "###### Mattermost GitHub Plugin - Slash Command Help\n" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ import ( | |
"github.com/mattermost/mattermost/server/public/pluginapi/experimental/bot/logger" | ||
"github.com/mattermost/mattermost/server/public/pluginapi/experimental/bot/poster" | ||
"github.com/mattermost/mattermost/server/public/pluginapi/experimental/telemetry" | ||
"github.com/mattermost/mattermost/server/public/pluginapi/i18n" | ||
|
||
"github.com/mattermost/mattermost-plugin-github/server/plugin/graphql" | ||
) | ||
|
@@ -95,6 +96,8 @@ type Plugin struct { | |
poster poster.Poster | ||
flowManager *FlowManager | ||
|
||
b *i18n.Bundle | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This names seems a bit cryptic to me. Maybe we can call it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, |
||
|
||
CommandHandlers map[string]CommandHandleFunc | ||
|
||
// githubPermalinkRegex is used to parse github permalinks in post messages. | ||
|
@@ -267,6 +270,12 @@ func (p *Plugin) OnActivate() error { | |
p.webhookBroker = NewWebhookBroker(p.sendGitHubPingEvent) | ||
p.oauthBroker = NewOAuthBroker(p.sendOAuthCompleteEvent) | ||
|
||
i18nBundle, err := i18n.InitBundle(p.API, filepath.Join("assets", "i18n")) | ||
if err != nil { | ||
return err | ||
} | ||
p.b = i18nBundle | ||
|
||
botID, err := p.client.Bot.EnsureBot(&model.Bot{ | ||
OwnerId: Manifest.Id, // Workaround to support older server version affected by https://github.com/mattermost/mattermost-server/pull/21560 | ||
Username: "github", | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious how we can use the translations in the templates?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah @mickmister I was trying to find the same thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we expose functions to call from within the template?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is something we should try and solve before merging the PR. Any question marks we have on how to do things like this should ideally be resolved before contributors go and work on the same sort of things
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can implement this method:
Then change
mattermost-plugin-github/server/plugin/template.go
Line 38 in e958064
to
and add a block here
mattermost-plugin-github/server/plugin/template.go
Lines 108 to 116 in e958064
to
Then we should be able to use the
i18n
function in the templatesThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @mickmister Thanks for the approach above, I tried a bunch of things as stated above but faced several issues.
First of all, there is a huge lack of documentation and resources on the internet regarding Go's text/template and i was not able to find anything which connects the translation and template together. If you have some resources, please share those i would love to deep dive into those.
First of all, When using higher-order functions inside the template directly, it prints the function returned by the parent function instead of trying to run the child function returned with the arguments passed(similar code here dummy code). I also tried storing the child function in a variable and calling it with the arguments, but that didn't work either. (You can have a link here goPlayground)
Secondly, the approach of making the initTemplate a method of the Plugin also didn't work because, during the initialization of the template, the Plugin is null.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Kshitij-Katiyar We should be able to call
initTemplate
duringOnActivate
to make that workI'm not sure about the template functions behavior