Add a script to oopen NVDA config directory

[lukaszgo1]

Link to issue number:

Closes #2214

Summary of the issue:

At the moment when someone wants to open config directory for NVDA he has to navigate there manually in case of portable versions, or use appropriate option from the Start Menu. It would be easier to do that by pressing a keyboard shortcut.

Description of how this pull request fixes the issue:

The functionality used to open config dir was moved from NVDA_slave to a separate function in the new systemUtils module. This function is used by unassigned by default script. User guide was updated accordingly.
As suggested by @feerrenrut I've also moved execElevated and hasUiAcces from the config to the systemUtils. To avoid breaking add-ons they still can be invoked from the config, but I've added a commend stating that they would be removed from there in NVDA 2021.1.

Testing performed:

• Assigned a gesture to the newly added script, and ensured that it works.
• Ensured that config directory can be opened from the start menu shortcut.
• Tested some places in which moved functions were used with a self-signed build in particular:
• Copying of the user config to the systemConfig
• Execution of COM registration fixing tool
• Enabling and disabling NVDA on a secure screen.

Known issues with pull request:

None known

Change log entry:

Section: New features,
New script without gesture assigned by default was added to open NVDA configuration directory from anywhere.

Section changes for developers:
execElevated and hasUiAcces are now moved to the new systemUtils module. They still can be invoked from config, but this possibility is going to be removed in NVDA 2021.1

[Brian1Gaff]

Does this have any security implications?

[lukaszgo1]

No, it does not. Both menu item and a keyboard command are not functional when on a secure desktop.

[XLTechie]

It is also worth discussing if directory containing configuration in case of portable copies can be called user configuration directory.

True question also for temporary copies. They are all generically the "NVDA Configuration Directory".

[lukaszgo1]

@LeonarddeR Do you still think that the name of this option should be changed?

[LeonarddeR]

I leave it up to Reef or Mick to decide on that.

[lukaszgo1]

As the only remaining think to be done before this can be merged is to decide on naming maybe you could take a quick look @feerrenrut

[feerrenrut]

I don't think this is appropriate for most users, as such I'm very reluctant for it to be an option in the tools menu. I would accept it added to the the advanced settings tab.

[Adriani90]

I agree also, this would be consistent with what we have for the developer scratchpad.

[lukaszgo1]

@feerrenrut wrote:

I don't think this is appropriate for most users, as such I'm very reluctant for it to be an option in the tools menu. I would accept it added to the the advanced settings tab.

While I understand that you are worried about less experienced users opening config directory by mistake we are already exposing it in the start menu, so it is hardly hidden. The tools menu also contains option to open a Python Console which, in the hand of an inexperienced user, can certainly be more disastrous than being able to look into the config. The problem with adding into the advanced tab of settings is that it cannot be accessed quickly and having a quick access to it was the main reason for #2214 to be created in the first place. As an alternative what about removing this option from the GUI completely and leaving the part of this PR which implements a unassigned script to open config directory? We are exposing some more advanced opttions such as WX inspection tool in this way already, so it seems a reasonable compromise between making it too easy for a beginner to corrupt his configuration, and giving power users easy access to it.

[feerrenrut]

I think there is a bit of tidying up necessary in those menus. As an aside, I'd be interested in introducing a 'developer mode' toggle on the advanced settings page, then when not checked hide the python console.

As an alternative what about removing this option from the GUI completely and leaving the part of this PR which implements a unassigned script to open config directory?

I'm happy with that approach, go ahead.

[lukaszgo1]

@feerrenrut Changed as discussed above.

[lukaszgo1]

@feerrenrut All your points are now addressed.

[lukaszgo1]

Yes, I've tested that the link is functional.

[feerrenrut]

Hi @lukaszgo1, have you installed this build and tested the open config directory option from the start menu as well?

[lukaszgo1]

@feerrenrut wrote:

have you installed this build and tested the open config directory option from the start menu as well?

Thanks for reminding me about this. Testing shows that in the current state it is not functional. The log is as follows:

ERROR - RPC process 2920 (nvda_slave.exe) (12:28:20.889) - Dummy-1 (2800):
__main__.main:
slave error
Traceback (most recent call last):
  File "nvda_slave.pyw", line 70, in main
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 668, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 638, in _load_backward_compatible
  File "ui.pyc", line 18, in <module>
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 668, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 638, in _load_backward_compatible
  File "gui\__init__.pyc", line 24, in <module>
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 668, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 638, in _load_backward_compatible
  File "speech\__init__.pyc", line 15, in <module>
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 668, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 638, in _load_backward_compatible
  File "api.pyc", line 12, in <module>
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 668, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 638, in _load_backward_compatible
  File "review.pyc", line 10, in <module>
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 668, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 638, in _load_backward_compatible
  File "NVDAObjects\__init__.pyc", line 16, in <module>
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 668, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 638, in _load_backward_compatible
  File "eventHandler.pyc", line 12, in <module>
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 668, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 638, in _load_backward_compatible
  File "treeInterceptorHandler.pyc", line 10, in <module>
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 668, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 638, in _load_backward_compatible
  File "documentBase.pyc", line 7, in <module>
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 668, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 638, in _load_backward_compatible
  File "scriptHandler.pyc", line 13, in <module>
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 668, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 638, in _load_backward_compatible
  File "sayAllHandler.pyc", line 8, in <module>
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 668, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 638, in _load_backward_compatible
  File "synthDriverHandler.pyc", line 18, in <module>
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 668, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 638, in _load_backward_compatible
  File "speechDictHandler\__init__.pyc", line 14, in <module>
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 668, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 638, in _load_backward_compatible
  File "speechDictHandler\dictFormatUpgrade.pyc", line 15, in <module>
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 668, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 638, in _load_backward_compatible
  File "speechDictHandler\speechDictVars.pyc", line 10, in <module>
AttributeError: 'NoneType' object has no attribute 'configPath'

I believe it is caused by the fact that when ui is imported it imports speech and it cannot be imported in a separate process. When the function responsible for opening config dir is in the config module as it was initially everything works as it should. Thinking about it some more I do not think that ui is a proper place for it - this module contains functions presenting messages to the user and slave works even when screen reader is not running. Should I try to make it work when the function is in ui module anyway, or would having it in config be acceptable?

[feerrenrut]

Should I try to make it work when the function is in ui module anyway, or would having it in config be acceptable?

No, I also had some concerns about putting it in UI. I still don't think it should live in config either, but looking at it more closely I see there are a number of "systemUtil" related things. In the future I think these should be moved to a new module. Perhaps you would like to do that in this PR? To maintain backwards compat you would have to import the names back into config and leave a comment to say it will be removed in the future. What do you think?

[lukaszgo1]

@feerrenrut wrote:

No, I also had some concerns about putting it in UI. I still don't think it should live in config either, but looking at it more closely I see there are a number of "systemUtil" related things. In the future I think these should be moved to a new module. Perhaps you would like to do that in this PR? To maintain backwards compat you would have to import the names back into config and leave a comment to say it will be removed in the future. What do you think?

Could you please give some examples of functions which in your opinion should be moved, just to ensure that we are on the same track? While I have no problem with doing this, I am wondering if it gives any real advantages, except, of course, more logical code layout.

[feerrenrut]

The main advantage in my opinion is to de-tangle the dependencies, making it easier to understand and verify the correctness of the code.

Looking through the files, the most obvious candidates are:

• def execElevated
• def hasUiAccess
• This new method.

If you do this, you'll have to avoid a circular reference. Since config will need to depend on this module (for execElevated) you'll need to remove dependencies on config from the new module. You can do this by making your new method more general EG def OpenDirectory(path) it can then be used like: OpenDirectory(path=config.GetUserConfigPath()) or similar.

I've only looked at this quickly, there are likely details I've missed. Hopefully this gives the general idea.

[lukaszgo1]

@feerrenrut Sory for the delay. This is ready for another look. I've updated description accordingly.

[feerrenrut]

Thanks @lukaszgo1 this looks good.

the changes have been addressed

[kvark128]

The systemUtils.openUserConfigurationDirectory function uses config.getUserDefaultConfigPath, which always returns the default path. If the configuration path was changed via the command line, it will open wrong directory.

[lukaszgo1]

@kvark128 Thanks for your commend. I've created #10911 to address this.