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 main package error loop when giving nonexistent parameter #3463

Closed
nvaccessAuto opened this issue Aug 23, 2013 · 16 comments
Closed

NVDA main package error loop when giving nonexistent parameter #3463

nvaccessAuto opened this issue Aug 23, 2013 · 16 comments

Comments

@nvaccessAuto
Copy link

Reported by leonarddr on 2013-08-23 10:56
When starting the NVDA main package with a nonexistent parameter (such as --dog), the introductory sound is heard and the program starts to throw the following error messages:

Error

Usage: nvda_noUIAccess.exe [options]error: no such option: --dog

Pressing OK, the dialogue disappears and reappears until i manually close the main package using task manager.

@nvaccessAuto
Copy link
Author

Comment 1 by jteh on 2013-08-23 11:04
Annoying, but non-standard, unsupported usage. :)

@nvaccessAuto
Copy link
Author

Comment 2 by leonarddr on 2013-08-23 11:10
It could be fixed quite easily i assume though, since --help doesn't cause similar behavior.

@nvaccessAuto
Copy link
Author

Comment 3 by jteh on 2013-08-23 11:16
The launcher watches for exit code 2 in order to allow NVDA to restart (e.g. due to add-on installation or language change). Otherwise, the launcher would think NVDA had exited and try to clean up the temporary copy. The problem is that a command line error also uses exit code 2.

@nvaccessAuto
Copy link
Author

Comment 4 by nvdakor on 2013-08-23 12:20
Hi,
A few questions:

  • Is this an installed or a portable copy?
  • Is this from a 32-bit or 64-bit? (this might be helpful in some situations, but since NVDA itself is a 32-bit executable, it may not matter).
    Thanks.

@nvaccessAuto
Copy link
Author

Comment 5 by leonarddr (in reply to comment 4) on 2013-08-23 12:23
Replying to nvdakor:

  • Is this an installed or a portable copy?

It is the NVDA Main Package for NVDA 2013.2 RC2, with the "--dog" flag, so a nonexistent one. It's thus neither an isntalled nor a portable copy.

  • Is this from a 32-bit or 64-bit? (this might be helpful in some situations, but since NVDA itself is a 32-bit executable, it may not matter).

It's an X64 system.

@nvaccessAuto
Copy link
Author

Comment 6 by jteh on 2013-08-23 12:25
To clarify, there's no need to request/provide more info. The explanation (and therefore the way to fix) is given in comment:3.

@nvaccessAuto
Copy link
Author

Comment 7 by nvdakor on 2013-08-23 13:03
Hi,
I guess the trivial fix would be detecting that we're using installer i.e. temp copy and throw exit(1) exception instead (what might be happeing, as Jamie said and if I got the description right, is that the installer is stuck with the wrong args).

@nvaccessAuto
Copy link
Author

Comment 8 by leonarddr on 2013-09-02 15:59
I'd say that the exit code of command line errors (error function) should be changed to another value, for example exit code 3. sys.exit(2) in nvda.pyw could be changed accordingly, unless that creates other issues.
Exit code change could also be made in core.py, restart function. I think this one would be better, since exit code 2 is launcher-specific there. This also would require a change in launcher code: intcmp $1 2 exec +1

@nvaccessAuto
Copy link
Author

Comment 9 by PZajda on 2013-09-13 18:36
Hi,

I done and used the second solution.

To test it, I tried:

  • To pass a wrong command line to the launcher. The message was displayed only once.
  • To change the language while I used the launcher. NVDA restarted correctly.
  • To install NVDA, then when it was finished and started, I pressed Alt+Ctrl+n then NVDA restarted correctly.
  • At last, I tried to pass a wrong command line argument to %programfiles%\nvda\nvda.exe and the command line error message was displayed, I prefered to test it even if it is not really an useful test.

It is available on git.
Repository: https://bitbucket.org/pzajda/nvda.git
Branch: t3463

@nvaccessAuto
Copy link
Author

Comment 10 by James Teh <jamie@... on 2013-09-15 15:31
In [a668532]:

If an unknow command line argument is passed to the NVDA distribution package, NVDA no longer displays error message dialogs endlessly.

This occurred because command line errors are signalled with exit code 2, but the same exit code was also being used to signal NVDA restarts to the launcher.
Now, restarts are signalled to the launcher using exit code 3.
Authors: Patrick ZAJDA patrick@zajda.fr, James Teh jamie@nvaccess.org
Re #3463.

@nvaccessAuto
Copy link
Author

Comment 11 by jteh on 2013-09-15 15:34
Thanks again for your work!

Code review (which i addressed in the branch I just pushed):

  • You forgot to update the comment in nvdaLauncher.nsi about exit code 2.
  • The exit codes in nvda_slave.pyw are different and shouldn't be changed.
  • The commit message should ideally be more descriptive for a change like this.
    Changes:
    Milestone changed from None to next

@nvaccessAuto
Copy link
Author

Comment 12 by James Teh <jamie@... on 2013-09-15 15:35
In [c41367a]:

Merge branch 't3463' into next

Incubates #3463.

Changes:
Added labels: incubating

@nvaccessAuto
Copy link
Author

Comment 13 by Michael Curran <mick@... on 2013-10-02 01:44
In [59a04da]:

If an unknow command line argument is passed to the NVDA distribution package, NVDA no longer displays error message dialogs endlessly.

This occurred because command line errors are signalled with exit code 2, but the same exit code was also being used to signal NVDA restarts to the launcher.
Now, restarts are signalled to the launcher using exit code 3.
Authors: Patrick ZAJDA patrick@zajda.fr, James Teh jamie@nvaccess.org
Fixes #3463.

Changes:
Removed labels: incubating
State: closed

@nvaccessAuto
Copy link
Author

Comment 14 by mdcurran on 2013-10-02 01:44
Changes:
Milestone changed from next to 2013.3

@nvaccessAuto
Copy link
Author

Comment 15 by leonarddr on 2013-10-11 06:38
De changelog for this entry contains a typo, swiwtch rather than switch. I'm however not able to change it as the changelog what's new entry isn't included in this ticket.

@nvaccessAuto
Copy link
Author

Comment 16 by jteh on 2013-10-22 23:05
Correcting version, since 2013.2 wasn't previously an option. See #3499.

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

No branches or pull requests

2 participants