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

NVDA should warn about unknown command line parameters #12795

Closed
lukaszgo1 opened this issue Sep 1, 2021 · 4 comments · Fixed by #13087
Closed

NVDA should warn about unknown command line parameters #12795

lukaszgo1 opened this issue Sep 1, 2021 · 4 comments · Fixed by #13087
Milestone

Comments

@lukaszgo1
Copy link
Contributor

lukaszgo1 commented Sep 1, 2021

First of the series of issues created as a result of discussion in #12076

Is your feature request related to a problem? Please describe.

Currently NVDA does not warn when it was started with invalid command line parameters. For example if someone misspells --disable-addons and attempts to rule that the given issue is caused by add-ons or not there is no indication that testing they're doing does not prove anything.
This problem exists because all CLI arguments unknown to NVDA are placed in globalVars.appArgsExtra (the motivation for this is to allow plugins to support additional CLI arguments)

Describe the solution you'd like

NVDA should warn on startup about arguments which are unknown and which has not been used by add-ons. To do so it would be necessary to make plugins register for parameters they're interested in rather than just passively looking for them in globalVars.appArgsExtra.
To ensure that the new solution is as flexible as old globalVars.appArgsExtra were the following points need to be considered:

  • First plugin which is interested in the given parameter does not remove it from the list of unknown params - the old system allowed two plugins to share parameter with the same name

The design which I have in mint is as follows:
-In addonHandler there is new extensionPoint CLIParamIsKnown to which all globalPlugins interested in processing command line parameters should register.

  • Unfortunately none of the extensionsPoints we have currently would work for this. Filter modifies data thereby removing a CLI param making it impossible for later plugin to process the same parameter) and Decider returns as soon as first interested plugin would be interested in the given parameter. The ideal option would be to create a Decider which accumulates results and returns True if at least one handler returns True, False otherwise.

After NVDA starts each argument placed in globalVars.appArgsExtra is provided to all handlers registered to the extension point - if at least one parameter is not known to add-ons core.doStartupDialogs should show a dialog informing about what unknown parameters were provided. The disadvantage of this approach is that all add-ons which want to parse extra command line parameters have to ship a global plugin (there is no guarantee that app module or other type of plugin would be loaded on startup).

Describe alternatives you've considered

  • Leave code as is
  • Do not allow plugins to support command line parameters (I'm not aware about any which needs this functionality). @LeonarddeR Since globalVars.appArgsExtra were introduced by you in Switch from optparser to argparser for parsing command line arguments #6865 would you be able to tell if you know about any use cases or has this been implemented just in case some plugin may need this? I'll also ask about possible usages on add-ons list.

Additional context

@lukaszgo1
Copy link
Contributor Author

@feerrenrut
Copy link
Contributor

Can you expand on how this would alter the process of startup? At the moment, parsing args happens before add-ons are loaded. I think it has to be that way. This means that unknown args will be flagged before add-ons have a chance to register them as accepted, or am I missing something?

@lukaszgo1
Copy link
Contributor Author

lukaszgo1 commented Sep 2, 2021

Can you expand on how this would alter the process of startup? At the moment, parsing args happens before add-ons are loaded. I think it has to be that way. This means that unknown args will be flagged before add-ons have a chance to register them as accepted, or am I missing something?

I've updated the description hopefully addressing these concerns.

@feerrenrut
Copy link
Contributor

Yes, that helped. Another option might be to require addon args to have a known prefix, EG --addon.doX

seanbudd pushed a commit that referenced this issue Nov 24, 2021
Closes #12795

Summary of the issue:
When starting NVDA with unknown (not used by NVDA nor by add-ons)
command line parameters there is no warning.

Description of how this pull request fixes the issue:
Introduced a new extension point which accumulates decisions made by all handlers and allowed to register add-ons for command line arguments they want to use. Note that if the given add-on contains only app modules it has to bundle a small global plugin to make sure that it is loaded on NVDA's startup.
On NVDA startup user is warned if there are any arguments which are unknown to NVDA and not used by add-ons.
@nvaccessAuto nvaccessAuto added this to the 2022.1 milestone Nov 24, 2021
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 a pull request may close this issue.

3 participants