Skip to content

Command line switch to specify language on startup (#10044) #10089

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.

Sign up for GitHub

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

Closed
JulienCochuyt wants to merge 4 commits into from
Closed

Command line switch to specify language on startup (#10044) #10089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7611b13
Command line switch to specify language on startup (#10044)
JulienCochuyt Aug 13, 2019
a852fa6
Validation of the `--lang` command line parameter (#10044)
JulienCochuyt Sep 7, 2019
113ae13
Merge branch 'master' into i10044-cmdLineLanguage
JulienCochuyt Nov 18, 2019
1fce583
Merge branch 'master' into i10044-cmdLineLanguage
JulienCochuyt May 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions source/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,13 @@ def restart(disableAddons=False, debugLogging=False):
sys.argv.remove("--ease-of-access")
except ValueError:
pass
if globalVars.appArgs.cmdLineLanguage:

@LeonarddeR LeonarddeR Aug 29, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think cmdLanguage can just be language. globalVars.appArgs already implies that we're dealing with a command line argument here. Having said that, I note that some magic is happening with the configPath, but that could be considered confusing.

@JulienCochuyt JulienCochuyt Sep 7, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think cmdLanguage can just be language. globalVars.appArgs already implies that we're dealing with a command line argument here.

If i'm not wrong, other members of globalVars.appArgs convey their default value if not specified from the command line. This one is different as, if not specified on the command line, it would not be set to the value in the configuration. Thus, I chose that name to be extra explicit and ensure no one mistakes the intent behind this member.
Let me know if you feel like my reasoning is wrong here and I will rename it as you suggest.

Having said that, I note that some magic is happening with the configPath, but that could be considered confusing.

I don't get your point here. Could you please elaborate?

@LeonarddeR LeonarddeR Sep 11, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Having said that, I note that some magic is happening with the configPath, but that could be considered confusing.

I don't get your point here. Could you please elaborate?

If you do not start NVDA with the --config-path parameter, globalVars.appArgs.configPath is still populated with the actual config path. It is also widely used throughout the code base to refer to the config path. A similar thing could happen with globalVars.appArgs.language in that if it is not overridden using the command line, can be set to the actual language.

Just a suggestion: Isn't it possible to show up a simple question on startup if NVDA was started with a command line flag?

I think this suggestion makes sense, as it will address the concern about what is displayed in the general settings panel for the language.

import config
# We should better compare here with the saved value if not saveOnExit.
if config.conf["general"]["language"] != globalVars.appArgs.cmdLineLanguage:
for i, arg in list(enumerate(sys.argv)):
if arg.startswith("--lang="):
del sys.argv[i]
shellapi.ShellExecute(None, None,
sys.executable,
subprocess.list2cmdline(sys.argv + options),
Expand Down Expand Up @@ -165,7 +172,12 @@ def resetConfiguration(factoryDefaults=False):
config.conf.reset(factoryDefaults=factoryDefaults)
logHandler.setLogLevelFromConfig()
#Language
lang = config.conf["general"]["language"]
lang = globalVars.appArgs.cmdLineLanguage
if lang:
# Ensure the language specified on the command line will be saved with the config.

@LeonarddeR LeonarddeR Aug 29, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I"m not sure whether this is what people expect from command line arguments. They are most of the time only seen as temporary overrides. I think I prefer the language setting to be that way as well, i.e. it should only be effective to the current session of NVDA, until there is a restart triggered from outside that resets the params. It should not touch the configuration at all.

@JulienCochuyt JulienCochuyt Sep 7, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What happens if you don't provide the parameter and change the language when save on exit is disabled? Is it then properly updated?

(Before and after this PR, same process)
If one changes the language and answers "No" when asked to restart right away, the change is - as expected - lost when restarting if the configuration has not been saved.

@JulienCochuyt JulienCochuyt Sep 7, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It should not touch the configuration at all.

The original poster of #10144 accidentally changed language to Hebrew, a language which he cannot understand. To get him back on his feet, we need a way to change this setting.

The point is: If the language specified on the command line is different from the configuration, what should the settings dialog present as the selected language?

I see four options here:

  1. The language specified on the command line. Hence, if the configuration is saved (manually or automatically upon exit), the new language is applied. This is the approach I took.
  2. The original language of the configuration, which does not reflect the state NVDA is currently in. I felt it was quite awkward when testing.
  3. The language specified on the command line suffixed with "forced from command line". This is explicit, but also requires greater impact and I felt it was uselessly cumbersome.
  4. The original language of the configuration, but in a disabled drop-down preceded by a message explaining another language was specified on the command line and a button to allow for changing this setting. This is explicit, but also requires the greatest impact and I felt it was uselessly cumbersome.

What do you think? Would you prefer option 2?

@feerrenrut feerrenrut Jul 6, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In other situations where we override from the command-line we disable the control so that we avoid this confusion. That, of course, defeats the purpose of this PR.

My preference would be to insert a new item in the language list (at index 0) with something like command line option: en_au or similar.

  • If the user leaves this option, they can safely change other options and save their config without accidentally changing their language.
  • If the user explicitly changes the language to en_au then we know they wish to make it permanent.

config.conf["general"]["language"] = lang
else:
lang = config.conf["general"]["language"]
log.debug("setting language to %s"%lang)
languageHandler.setLanguage(lang)
# Addons
Expand Down Expand Up @@ -235,7 +247,12 @@ def main():
pass
logHandler.setLogLevelFromConfig()
try:
lang = config.conf["general"]["language"]
lang = globalVars.appArgs.cmdLineLanguage
if lang:
# Ensure the language specified on the command line will be saved with the config.
config.conf["general"]["language"] = lang
else:
lang = config.conf["general"]["language"]
import languageHandler
log.debug("setting language to %s"%lang)
languageHandler.setLanguage(lang)
Expand Down
28 changes: 28 additions & 0 deletions source/nvda.pyw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ if not winVersion.isSupportedOS():
winUser.MessageBox(0, ctypes.FormatError(winUser.ERROR_OLD_WIN_VERSION), None, winUser.MB_ICONERROR)
sys.exit(1)


def stringToBool(string):
"""Wrapper for configobj.validate.is_boolean to raise the proper exception for wrong values."""
from configobj.validate import is_boolean, ValidateError
Expand All @@ -81,6 +82,26 @@ def stringToBool(string):
except ValidateError as e:
raise argparse.ArgumentTypeError(e.message)


def stringToLang(value: str) -> str:
"""Perform basic case normalization for ease of use.
"""
if value.casefold() == "Windows".casefold():
return "Windows"
value = value.replace("-", "_")
lang = value.split("_")[0].lower()
dialect = value.split("_")[1].upper() if "_" in value else None
# Validating the size of the codes.
# Further validation would require L{languageHandler} to be initialized.
if len(lang) != 2 or (dialect and len(dialect) != 2):

@feerrenrut feerrenrut Jul 6, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would be great if this could check that the lang and dialect is in the list of supported languages. These errors could include the available languages.

raise argparse.ArgumentTypeError(
"Language code should be \"Windows\" or of the forms \"en\" or \"pt_BR\"."
)
if dialect:
return f"{lang}_{dialect}"
return lang


#Process option arguments
parser=NoConsoleOptionParser()
quitGroup = parser.add_mutually_exclusive_group()
Expand All @@ -89,6 +110,13 @@ parser.add_argument('-k','--check-running',action="store_true",dest='check_runni
parser.add_argument('-f','--log-file',dest='logFileName',type=str,help="The file where log messages should be written to")
parser.add_argument('-l','--log-level',dest='logLevel',type=int,default=0,choices=[10, 12, 15, 20, 30, 40, 50, 100],help="The lowest level of message logged (debug 10, input/output 12, debugwarning 15, info 20, warning 30, error 40, critical 50, off 100), default is info")
parser.add_argument('-c','--config-path',dest='configPath',default=None,type=str,help="The path where all settings for NVDA are stored")
parser.add_argument(
'--lang',
dest='cmdLineLanguage',
default=None,
type=stringToLang,
help="Override the configured NVDA language. Set to \"Windows\" for current user default, \"en\" for English, etc."
)
parser.add_argument('-m','--minimal',action="store_true",dest='minimal',default=False,help="No sounds, no interface, no start message etc")
parser.add_argument('-s','--secure',action="store_true",dest='secure',default=False,help="Secure mode (disable Python console)")
parser.add_argument('--disable-addons',action="store_true",dest='disableAddons',default=False,help="Disable all add-ons")
Expand Down
1 change: 1 addition & 0 deletions user_docs/en/userGuide.t2t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3068,6 +3068,7 @@ Following are the command line options for NVDA:
| -f LOGFILENAME | --log-file=LOGFILENAME | The file where log messages should be written to |
| -l LOGLEVEL | --log-level=LOGLEVEL | The lowest level of message logged (debug 10, input/output 12, debug warning 15, info 20, warning 30, error 40, critical 50, disabled 100), default is warning |
| -c CONFIGPATH | --config-path=CONFIGPATH | The path where all settings for NVDA are stored |
| None | --lang=LANGUAGE | Override the configured NVDA language. Set to "Windows" for current user default, "en" for English, etc. |
| -m | --minimal | No sounds, no interface, no start message, etc. |
| -s | --secure | Secure mode: disables Python console, profile features such as creation, deletion, renaming profiles etc., update check, some checkboxes in the welcome dialog and in general settings category (e.g. start NVDA after sign-in, save configuration after exit etc.), as well as logviewer and logging features (used often in secure screens). Note also that this command will disable the possibility to save settings in system config and the gesture map will not be saved on the disk. |
| None | --disable-addons | Addons will have no effect |
Expand Down