Added a view basic translation functions and translation file #137

mjarkk · 2018-08-13T13:43:39Z

What do you think of this way of translations?
Currently it doesn't auto detect the system language because i don't know how to detect that but the translation works when you change the language.English to language.Dutch

jesseduffield · 2018-08-13T14:03:58Z

Looks good to me! I've got a couple questions though:

Does this require the translation file to be sitting in the user's computer at runtime, or is it baked into the binary at compile time? If it's the former, I'm not sure how best to install that. If the user installs via go-get, that's easy, but it's harder for e.g. direct download from the binary. I'm not sure what the general pattern is for non-english speakers when it comes to these things. Is there usually a translation file that is available alongside the release for the user to download and store somewhere? Or is there some other approach

ponsfrilus · 2018-08-13T15:07:28Z

IMHO these should be translated too, as well as the error messages.

mjarkk · 2018-08-13T17:15:17Z

@jesseduffield Most programs have all translations in the binary by default.
For the Toml files they are not included in the binary so that's not an option anymore
I saw that go-i18n also has a view functions to add translations directly inside the code so that's probably the solution for the Toml files.
Another thing that may work is to use separate Go files that just exports that config file as a string but that would break the syntax highlighting for whats inside that string.

@ponsfrilus Thx for the suggestions i'll see what i can do

jesseduffield · 2018-08-13T23:06:02Z

@mjarkk this is fantastic work man I really appreciate the effort. I've just merged a big refactor PR #132 into master, meaning you might need to shuffle some things around to make it mergeable. It's only the first step of the refactor process so it's got some flaws but it removes things like global constants and generally improves the structure of the project. Let me know if you need any help merging with the new master branch

Update to latest master

mjarkk · 2018-08-14T09:38:14Z

I have fixed all the merge conflicts

jesseduffield · 2018-08-14T11:07:19Z

pkg/i18n/i18n.go

@@ -9,12 +9,16 @@ import (
 // the function to setup the localizer
 func getlocalizer() *i18n.Localizer {

+	// detect the user's language
 	userLang, _ := jibber_jabber.DetectLanguage()


jesseduffield · 2018-08-14T12:13:32Z

@mjarkk I've just pulled down this branch and have made a couple of changes so that it conforms to the project structure, namely attaching the localizer to the App and Gui structs so that we're not depending on global variables at runtime. Otherwise fantastic job! I'll push my changes now

jesseduffield · 2018-08-14T12:21:48Z

I've just realised that we need localization for error messages too. I think that's fixed by just attaching them to the gui as well which I can make a start on now

Updated to latest master

… struct

jesseduffield · 2018-08-14T13:51:42Z

I've just had to do some reading because I considered just switching back to the package-scoped localizer variable that was there initially so that we can still define our errors at the package level. After that reading it looks like it's still a bad idea to go down that route because functions are far more testable when their only input is their arguments and the properties of their structs. But that means we need to fix up how errors work right now. I think that as a general pattern we should just define errors inline and in the event that we use errors as sentinel values we'll need to define it on the gui struct (ideally we want to minimise use of sentinel errors)

the articles I've been reading are
https://dave.cheney.net/2017/06/11/go-without-package-scoped-variables
https://dave.cheney.net/2014/12/24/inspecting-errors
https://dave.cheney.net/2016/04/07/constant-errors

I've just pushed some more changes. I'm thinking that now that we can't define our error messages on the package-scope, maybe we should consider using a i18n bundle for storing defaults rather than stating the default in the localize function, otherwise we'll be duplicating a lot of text.

edit: if I get the time I'll switch to checking on error type like in here https://github.com/golang/go/wiki/Errors rather than storing these on the Gui

ghost · 2018-08-14T14:12:44Z

Not to hijack the thread, but there is something else to take into consideration. Both #95 and #89 are due to indexing on bytes instead of runes in https://github.com/jesseduffield/lazygit/blob/master/pkg/git/branch_list_builder.go#L144. This can be remedied quite easily, but then you must also provide a localization for abbreviatedTimeUnit.

mjarkk · 2018-08-15T12:44:58Z

The pkg/gui folder is now fully translated.
I'll later add an English translation file that can be used as template for other translations

Update to latest master

ponsfrilus · 2018-08-15T13:50:24Z

The English translation file as a template is a great idea! Thanks for doing that!

mjarkk · 2018-08-15T13:57:21Z

Jup that's what i thought to :)
It might be a bit annoying to add every ID and sentence 2 times but that would make it much easier to updates translation files and have a template to start with when making a translation for a new language

dawidd6 · 2018-08-15T18:53:57Z

Is it possible to use some kind of translation management system like https://weblate.org with this translation implemetation?

mjarkk · 2018-08-15T19:20:37Z

I'm not sure i need to look into that.

I have switched from Toml files for translation to just Go files because they are included in the build binaries because of that it (probably) isn't possible to use a translation service

It might be possible to do some magic with a file converter to convert all the .toml files to .go files but that would create 2 copies of everything

mjarkk · 2018-08-15T19:46:38Z

I have maybe found a solution to include Toml files inside the build binaries
It is possible using a tool from gobuffalo: https://github.com/gobuffalo/packr
But when using this you can't use go install anymore it needs to be: packr insatll
This tool might also increase the install output file with a big around like (1mb) that's around 30% more data

dawidd6 · 2018-08-15T20:00:15Z

I would go for first solution: .go files. But we need to think about maintainability.

Translators would need to be notified somehow about new entries available to translate (maybe permament github issue?).

jesseduffield · 2018-08-15T23:35:32Z

Fantastic work man. I agree with using an english translation file, but I think we should take it one step further and not have defaults against the inline-translations. This means if we ever use the same message in different places (as is the case with some error messages) we don't need to duplicate the default string in two places: we can just say gui.Tr.SLocalize(key) and we'll ensure the key is in the english (default) translation file. Then when at startup we initialize the localizer we'll merge the default into whatever specific localization file we're using. What are people's thoughts on this approach?

mjarkk · 2018-08-16T05:19:21Z

Removed all inline translations from gui.Tr.SLocalize and gui.Tr.TemplateLocalize

jesseduffield · 2018-08-16T08:20:01Z

pkg/gui/branches_panel.go

-	gui.createPromptPanel(g, v, "New Branch Name (Branch is off of "+branch.Name+")", func(g *gocui.Gui, v *gocui.View) error {
+	message := gui.Tr.TemplateLocalize(
+		"NewBranchNameBranchOff",
+		map[string]interface{}{


does this need to be map[string]interface{}? Are we able to say map[string]string?

Acording to go-i18n yes: https://github.com/nicksnyder/go-i18n#package-i18n-
But i haven't tested something else

We can create a custom type to use:

type Template map[string]interface{}

jesseduffield · 2018-08-16T08:27:14Z

pkg/gui/files_panel.go

 	}
 	sub, err := gui.GitCommand.AddPatch(file.Name)
 	if err != nil {
 		return err
 	}
 	gui.SubProcess = sub
-	return ErrSubProcess
+	return nil


can we make this return gui.Errors.ErrSubProcess

I'll test that out and see if the go checker doesn't spam me with errors

jesseduffield · 2018-08-16T08:27:46Z

pkg/gui/files_panel.go

@@ -222,15 +223,15 @@ func (gui *Gui) PrepareSubProcess(g *gocui.Gui, commands ...string) error {
 	}
 	gui.SubProcess = sub
 	g.Update(func(g *gocui.Gui) error {
-		return ErrSubProcess
+		return nil


same thing here

jesseduffield · 2018-08-16T08:27:53Z

pkg/gui/files_panel.go

@@ -241,7 +242,7 @@ func (gui *Gui) genericFileOpen(g *gocui.Gui, v *gocui.View, open func(string) (
 	}
 	if sub != nil {
 		gui.SubProcess = sub
-		return ErrSubProcess
+		return nil


jesseduffield

I left a couple comments. After those are addressed, I'm happy to merge :)

mjarkk · 2018-08-16T09:34:12Z

I hope this fixes this things you asked for :)

jesseduffield · 2018-08-16T10:52:43Z

pkg/gui/gui.go

@@ -49,6 +49,9 @@ func (gui *Gui) GenerateSentinelErrors() {
 	}
 }

+// Teml is short for template used to make the required map[string]interface{} shorter when using gui.Tr.SLocalize and gui.Tr.TemplateLocalize


can we put this in the i18n package and then call it as i18n.Temp, just incase we need to call it from another package as well

jesseduffield

just one thing :)

jesseduffield · 2018-08-16T10:54:30Z

pkg/gui/gui.go

+	}
+}
+
+// Teml is short for template used to make the required map[string]interface{} shorter when using gui.Tr.SLocalize and gui.Tr.TemplateLocalize


can we move this to the i18n package and then call it like i18n.Teml so that we can can access it from any package, not just gui?

jesseduffield · 2018-08-16T11:52:23Z

@mjarkk thanks again for all the effort you put into this PR it is fantastic work

mjarkk · 2018-08-16T11:57:26Z

NP i really like you're project :)

Added a view basic translation functions + tranlation file

62fc831

mjarkk added 3 commits August 13, 2018 20:46

fixed @jesseduffield comment #137

d9959eb

fixed typo

65eb378

Added more messages and text issue: #137

f2dfcb6

mjarkk added 5 commits August 14, 2018 11:05

tried to update to latest master

dfafb98

Merge pull request #1 from jesseduffield/master

17dcfbc

Update to latest master

Added the translation to some words again

5ad97ad

Merge remote-tracking branch 'origin/master'

f792f74

Added all the missing translations from dutch.go

dd7e93a

mjarkk added 2 commits August 14, 2018 12:52

Added auto detection for the system language

0c39347

added some commands

6e51814

mjarkk mentioned this pull request Aug 14, 2018

I want to help make i18n happen #134

Closed

jesseduffield reviewed Aug 14, 2018

View reviewed changes

mjarkk and others added 3 commits August 14, 2018 13:31

Added more translations

0568b32

make local i18n package confirm to project structure

5cbacb0

Merge branch 'master' of https://github.com/mjarkk/lazygit

4d0702f

mjarkk and others added 4 commits August 14, 2018 15:10

Merge pull request #2 from jesseduffield/master

8e22d56

Updated to latest master

fixed package naming and added tr object to file_panel.go

73a1682

pull errors out of package scope and store sentinel errors on the gui…

ba2b6fb

… struct

Merge branch 'master' of https://github.com/mjarkk/lazygit

d923796

can't go any further because of an error

883f436

Translated pkg/gui/view_helpers.go

50b41bf

mjarkk added 3 commits August 15, 2018 14:57

Added all english translations to a file and fixed some typos

d00c46a

Merge pull request #4 from jesseduffield/master

9112278

Update to latest master

Fully translated pkg/gui/confirmation_panel.go

9abbfe5

mjarkk mentioned this pull request Aug 15, 2018

kiedy po polsku? #154

Closed

Fixed comment on issue #137 from @jesseduffield

88e1a81

jesseduffield reviewed Aug 16, 2018

View reviewed changes

jesseduffield requested changes Aug 16, 2018

View reviewed changes

mjarkk added 2 commits August 16, 2018 11:31

Fixed comments from jesseduffield on issue #137

9074650

Fixed comments from jesseduffield on issue #137

faf218f

jesseduffield reviewed Aug 16, 2018

View reviewed changes

jesseduffield requested changes Aug 16, 2018

View reviewed changes

Fixed it

fcf616b

jesseduffield merged commit 090537a into jesseduffield:master Aug 16, 2018

jesseduffield mentioned this pull request Aug 20, 2018

Add tests to i18n package #193

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added a view basic translation functions and translation file #137

Added a view basic translation functions and translation file #137

mjarkk commented Aug 13, 2018

jesseduffield commented Aug 13, 2018

ponsfrilus commented Aug 13, 2018 •

edited

Loading

mjarkk commented Aug 13, 2018 •

edited

Loading

jesseduffield commented Aug 13, 2018

mjarkk commented Aug 14, 2018

jesseduffield Aug 14, 2018

jesseduffield commented Aug 14, 2018

jesseduffield commented Aug 14, 2018

jesseduffield commented Aug 14, 2018 •

edited

Loading

ghost commented Aug 14, 2018

mjarkk commented Aug 15, 2018

ponsfrilus commented Aug 15, 2018

mjarkk commented Aug 15, 2018

dawidd6 commented Aug 15, 2018

mjarkk commented Aug 15, 2018

mjarkk commented Aug 15, 2018

dawidd6 commented Aug 15, 2018

jesseduffield commented Aug 15, 2018 •

edited

Loading

mjarkk commented Aug 16, 2018

jesseduffield Aug 16, 2018 •

edited

Loading

mjarkk Aug 16, 2018

mjarkk Aug 16, 2018

jesseduffield Aug 16, 2018

mjarkk Aug 16, 2018

jesseduffield Aug 16, 2018

jesseduffield Aug 16, 2018

jesseduffield left a comment

mjarkk commented Aug 16, 2018

jesseduffield Aug 16, 2018

jesseduffield left a comment

jesseduffield Aug 16, 2018

mjarkk Aug 16, 2018

jesseduffield commented Aug 16, 2018

mjarkk commented Aug 16, 2018

Added a view basic translation functions and translation file #137

Added a view basic translation functions and translation file #137

Conversation

mjarkk commented Aug 13, 2018

jesseduffield commented Aug 13, 2018

ponsfrilus commented Aug 13, 2018 • edited Loading

mjarkk commented Aug 13, 2018 • edited Loading

jesseduffield commented Aug 13, 2018

mjarkk commented Aug 14, 2018

Choose a reason for hiding this comment

jesseduffield commented Aug 14, 2018

jesseduffield commented Aug 14, 2018

jesseduffield commented Aug 14, 2018 • edited Loading

ghost commented Aug 14, 2018

mjarkk commented Aug 15, 2018

ponsfrilus commented Aug 15, 2018

mjarkk commented Aug 15, 2018

dawidd6 commented Aug 15, 2018

mjarkk commented Aug 15, 2018

mjarkk commented Aug 15, 2018

dawidd6 commented Aug 15, 2018

jesseduffield commented Aug 15, 2018 • edited Loading

mjarkk commented Aug 16, 2018

jesseduffield Aug 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jesseduffield left a comment

Choose a reason for hiding this comment

mjarkk commented Aug 16, 2018

Choose a reason for hiding this comment

jesseduffield left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jesseduffield commented Aug 16, 2018

mjarkk commented Aug 16, 2018

ponsfrilus commented Aug 13, 2018 •

edited

Loading

mjarkk commented Aug 13, 2018 •

edited

Loading

jesseduffield commented Aug 14, 2018 •

edited

Loading

jesseduffield commented Aug 15, 2018 •

edited

Loading

jesseduffield Aug 16, 2018 •

edited

Loading