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

feat: Language in config file #1493

Merged
merged 5 commits into from
Aug 9, 2023
Merged

feat: Language in config file #1493

merged 5 commits into from
Aug 9, 2023

Conversation

buhtz
Copy link
Member

@buhtz buhtz commented Aug 8, 2023

New key "global.language" in config file to customize the UI language.

Language codes are used as values. If empty (or key not present) the systems current locale is used. If "en" or unknown the sources strings (in English) are used as fallback.

No matter that it hurt me I decided against creating a unit test for "global.language" config value. I have a refactoring about the config-file handling in my mind which will ease up a lot of things including unit testing.

@@ -192,6 +192,9 @@ class Config(configfile.ConfigFileWithProfiles):
PLUGIN_MANAGER = pluginmanager.PluginManager()

def __init__(self, config_path=None, data_path=None):
# Note: The main profiles name here is translated using the systems
# current locale because the language code in the config file wasn't
# read yet.
configfile.ConfigFileWithProfiles.__init__(self, _('Main profile'))
Copy link
Member Author

Choose a reason for hiding this comment

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

In the future we should remove the ability to translate the first profiles name. A name as an identifier shouldn't be translated at all. Related to #1371

Copy link
Contributor

Choose a reason for hiding this comment

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

or even better: Why do we need a default profile at all?

common/config.py Show resolved Hide resolved
@aryoda
Copy link
Contributor

aryoda commented Aug 8, 2023

I did not yet deep dive into the change set but AFAIR changing the config file format (here: by adding a new entry) requires to increment the config file version in common/config.py (CONFIG_VERSION) so that BiT automatically updates older config files to the new format.

I couldn't see this in the change set (and this also possibly requires to update the config file format of unit test configs to avoid an unexpected output that fails some unit tests)...

@buhtz
Copy link
Member Author

buhtz commented Aug 8, 2023

but AFAIR changing the config file format (here: by adding a new entry) requires to increment the config file version

I'm aware of that version variable but I I don't see a need to increment it. Adding a new variable do not "change the format". It do not break existing behavior. And the variable do not need to be present in the config file because there is a default value for it.

Technically there is no need to update old config files if you do not modify the name or format of existing variables. If the line global.language is not present the default value is used. If the value is stored the first time it is just added to the config file. This is quit flexible.

@aryoda
Copy link
Contributor

aryoda commented Aug 8, 2023

Adding a new variable do not "change the format"

I think reusing the new config format with an older BiT version will no longer work (though I am not sure if this is a use case).

Anyhow I think it would be more clean to increment the config file version every time we change the format (even though it is non-destructive/additive).

I am not sure how Germar handled this in the past, perhaps browsing the change history of the version could help.

@buhtz
Copy link
Member Author

buhtz commented Aug 8, 2023

I think reusing the new config format with an older BiT version will no longer work (though I am not sure if this is a use case).

It is an unusual case but possible. I'll test that. But I'm quit deep into the code of config.py and configfile.py. In my understanding this won't be a problem. The class read only what it needs and ignores the rest.

Anyhow I think it would be more clean to increment the config file version every time we change the format (even though it is non-destructive/additive).

I don't think it is clean to even have a version number for config formats. That is my problem with the solution. I don't see a need for such a version number. There are other ways to handle modifications of variable names and similar things in a config file.

@buhtz
Copy link
Member Author

buhtz commented Aug 8, 2023

I'll test that.

I added two lines to my BIT 1.3.3 config file.

foo.bar=8
global.bähm=nein

No problem. I can read all snapshot profiles and also modify them. That two lines are still in the config file.

@@ -192,6 +192,9 @@ class Config(configfile.ConfigFileWithProfiles):
PLUGIN_MANAGER = pluginmanager.PluginManager()

def __init__(self, config_path=None, data_path=None):
# Note: The main profiles name here is translated using the systems
# current locale because the language code in the config file wasn't
# read yet.
configfile.ConfigFileWithProfiles.__init__(self, _('Main profile'))
Copy link
Contributor

Choose a reason for hiding this comment

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

or even better: Why do we need a default profile at all?

@buhtz buhtz merged commit e52c324 into bit-team:dev Aug 9, 2023
1 check passed
@buhtz buhtz deleted the feat/langconfig branch August 9, 2023 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants