Skip to content

Conversation

@srifqi
Copy link
Contributor

@srifqi srifqi commented Dec 18, 2014

Rebase of #1937

@PilzAdam
Copy link
Contributor

This is not correctly rebased (see the +<<<<<<< HEAD).

@srifqi
Copy link
Contributor Author

srifqi commented Dec 18, 2014

Oops, missed that!
Will fix it as soon as possible.

EDIT: Can I just delete the +<<<<<<< HEAD, =======, >>>>>>> patch-4 and the old code (above =======)?

@PilzAdam
Copy link
Contributor

Don't you test your code after rebasing? You should...

@ShadowNinja
Copy link
Contributor

Please don't open new pull requests when rebasing.

@srifqi
Copy link
Contributor Author

srifqi commented Dec 20, 2014

@PilzAdam Sorry, didn't test it.
@ShadowNinja Sorry, don't know that. How to rebase existing pull request?

@Zeno- Zeno- removed the Rebase needed The PR needs to be rebased by its author label Dec 20, 2014
@srifqi
Copy link
Contributor Author

srifqi commented Dec 28, 2014

Okay, perfectly rebased. Thanks, @SmallJoker!

@srifqi
Copy link
Contributor Author

srifqi commented Feb 19, 2015

Okay, merge conflicts again. :(
Need code change for the NEW layout.

Copy link
Contributor

Choose a reason for hiding this comment

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

Space

@nerzhul
Copy link
Contributor

nerzhul commented Feb 19, 2015

If you rebase this could be good for merge. I agree with this change but i need to test it after rebase

@ShadowNinja ShadowNinja changed the title Add language setting Add language option in settings menu Feb 21, 2015
@srifqi
Copy link
Contributor Author

srifqi commented Mar 1, 2015

Okay, updated to fit new 0.4.12's layout.

PS: @SmallJoker please rebase this!

@SmallJoker SmallJoker force-pushed the add_language_setting branch 2 times, most recently from a6d554a to d7f2a2b Compare March 1, 2015 09:47
@srifqi
Copy link
Contributor Author

srifqi commented Mar 1, 2015

Thanks, @SmallJoker for the rebase!

Also, please update the translation (aka minetest.pot aka .po files) after merge.
As new word in this pr, Language:

EDIT: Found an error:
ERROR[main]: MSVC localization workaround active restating minetest in new environment!
Any chances of merge?

@rubenwardy
Copy link
Contributor

+1 to idea. A check box in the top right may be better, just white text until you click it.

@srifqi
Copy link
Contributor Author

srifqi commented Mar 5, 2015

@rubenwardy Sorry, what do you mean:

A check box in the top right may be better, just white text until you click it.

@rubenwardy
Copy link
Contributor

language

@srifqi
Copy link
Contributor Author

srifqi commented Mar 6, 2015

Okay, @rubenwardy
Thanks for ideas!
I'll update ASAP.

@Wuzzy2
Copy link
Contributor

Wuzzy2 commented Mar 6, 2015

I am opposed to this feature unless it allows the user to select “System language” as well (which just clears the language setting in minetest.conf), and this must be pre-selected at the first start.
So that Minetest will still start automatically in the system language in a sanely-configured operating system.

@rubenwardy
Copy link
Contributor

@Wuzzy, of course that will still be the case.
On 6 Mar 2015 09:55, "Wuzzy" notifications@github.com wrote:

I am opposed to this feature unless it allows the user to select “System
language” as well (which just clears the language setting in minetest.conf),
and this must be pre-selected at the first start.
So that Minetest will still start automatically in the system language in
a sanely-configured operating system.


Reply to this email directly or view it on GitHub
#1990 (comment).

@Wuzzy2
Copy link
Contributor

Wuzzy2 commented Mar 6, 2015

Good. :-)

@srifqi
Copy link
Contributor Author

srifqi commented Mar 7, 2015

@Wuzzy2, another good idea!
Let me check how to do that, or maybe any thoughts?

@srifqi
Copy link
Contributor Author

srifqi commented Mar 12, 2015

@Wuzzy2 @rubenwardy
Sorry, do not know how to that maybe in another PR.
So, this PR is final. Merge?

@srifqi srifqi force-pushed the add_language_setting branch from d7f2a2b to 71fd338 Compare March 26, 2015 12:46
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't account for things like the LANG env var which init_gettext does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ShadowNinja How to account things like the LANG env var?

EDIT: answered by @rubenwardy by using os.getenv()

makes the default selected language is based on LANG enviroment variable
@srifqi
Copy link
Contributor Author

srifqi commented Mar 31, 2015

Okay, @ShadowNinja
Updated, as your request.
Will be squashed soon when fixed.

@rubenwardy
Copy link
Contributor

Is it possible that os.getevn("LANG") returns nil?

@srifqi
Copy link
Contributor Author

srifqi commented Mar 31, 2015

@rubenwardy Haven't tested. Should I add if statement?

@Wuzzy2
Copy link
Contributor

Wuzzy2 commented Mar 31, 2015

Is it possible that os.getevn("LANG") returns nil?

No, this code does not even work. :P

If you meant os.getenv("LANG"), then the answer is “Yes.”. Because environment variables such as LANG can certainly be not defined at all. Next time, refer to the Lua documentation. :P
But in the case of LANG, it is most likely a sign of a poorly configured system if this environment variable is not set.

@srifqi
Copy link
Contributor Author

srifqi commented Mar 31, 2015

Then, should I add if statement?
Like this:

local LANG_env = os.getenv("LANG")
if LANG_env  == nil or LANG_env  == "" then
    LANG_env = "en"
end

@rubenwardy
Copy link
Contributor

Wuzzy, that was obviously a typo. I read the documentation today when the
author asked how to read environment variables in #minetest-mods. I don't
appreciate your tone.

You might be able to do: (pseudo code)

setting or envLANG or "en"

Instead of an if statement, but I've never tried three in a row like that.

@Wuzzy2
Copy link
Contributor

Wuzzy2 commented Mar 31, 2015

I am not sure if it is so simple by just reading LANG and applying the value directly, even with nil check. I am not very familar with locales and stuff, sorry. :-(

For example, on my system, LANG contains de_DE.utf8.

@mahmutelmas06
Copy link
Contributor

Its better to restart game when language changed instead of showing a message.

@MoNTE48
Copy link
Contributor

MoNTE48 commented Sep 11, 2015

2015-09-11 08:21:12: ERROR[main]: ========== ERROR FROM LUA ===========
2015-09-11 08:21:12: ERROR[main]: Failed to load and run script from 
2015-09-11 08:21:12: ERROR[main]: C:\Users\monte\Desktop\minetest-0.4.13\bin\..\builtin\init.lua:
2015-09-11 08:21:12: ERROR[main]: ...minetest-0.4.13\bin\..\builtin\mainmenu\tab_settings.lua:196: attempt to call field 'get_dirlist' (a nil value)
2015-09-11 08:21:12: ERROR[main]: stack traceback:
2015-09-11 08:21:12: ERROR[main]:   ...minetest-0.4.13\bin\..\builtin\mainmenu\tab_settings.lua:196: in main chunk
2015-09-11 08:21:12: ERROR[main]:   [C]: in function 'dofile'
2015-09-11 08:21:12: ERROR[main]:   ...Desktop\minetest-0.4.13\bin\..\builtin\mainmenu\init.lua:43: in main chunk
2015-09-11 08:21:12: ERROR[main]:   [C]: in function 'dofile'
2015-09-11 08:21:12: ERROR[main]:   ...rs\monte\Desktop\minetest-0.4.13\bin\..\builtin\init.lua:31: in main chunk
2015-09-11 08:21:12: ERROR[main]: ======= END OF ERROR FROM LUA ========
2015-09-11 08:21:12: ERROR[main]: No future without mainmenu

@kilbith
Copy link
Contributor

kilbith commented Oct 17, 2015

@PilzAdam You can close this.

@PilzAdam
Copy link
Contributor

006ef5b
This is the 6th issue closed by this commit. I'm really productive :D

@PilzAdam PilzAdam closed this Oct 18, 2015
@srifqi
Copy link
Contributor Author

srifqi commented Oct 22, 2015

Okay,
At least, thanks for responding here.

Off-topic: But, I a little bit don't like the tree-like settings, How about more dialog? ... and some presets?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants