Comment 2 by nvdakor on 2014-04-16 18:49
Hi,
In core loop, If config.conf can be read after config.initialize(), then we can implement the ticket as follows:

1. Add a new config variable named "playStartupAndExitSound" and place it in the general settings dialog. This option should be on by default.
2. If NVDA is run without minimal mode or if the option above is on, play the startup/exit file.
   Few things:

• This should not be done in secure screens.
  Thanks.

Comment 3 by nvdakor on 2014-04-16 20:02
Hi,
Initial back-end work in place (the GUI for this setting will be added after code review):

• Branch: t834
• Commit ID: da88d67
  Added a new config.conf setting named playStartAndExitSound which is on by default, as well as added a check for this setting in core loop.
  Thanks.

Comment 5 by nvdakor on 2014-04-18 06:04
Hi,
The front-end (GUI) and documentation has been added:

• Branch: t834
• Commit ID: 6a5a60b
  Hope I edited the correct section in changes.t2t. Thanks.

Comment 6 by nvdakor on 2014-04-19 09:04
Hi all,
For those who'd like to experiment with this can try this build:
http://community.nvda-project.org/try/t834/nvda_snapshot_try-t834-9992,9f3d48b.exe
There is a new setting in General Settings named "Play sound when starting or exiting NVDA" which is checked by default. If you uncheck this, no piano sound will be heard when you start or exit NVDA.
Good luck.

Comment 7 by jteh on 2014-05-26 06:52
Thanks Joseph. I'm happy with this for the most part. Minor things for you to fix:

• The config name "playStartAndExitSound" should probably be "playStartAndExitSounds".
• Just after your change to gui/settingsDialogs.py, there's a blank line that probably shouldn't be there.
• The What's New entry should just be noted in the "Changes document entry" field of the ticket. It'll only be added once we merge to master. We don't commit What's New entries to topic branches, as this was causing irritating merge conflicts.

Comment 8 by Joseph Lee <joseph.lee22590@... on 2014-05-26 07:42
In [80277e3]:

Conf and GUI/Settings: renamed config item and removed what's new entry (review by jteh). re #834.

Comment 9 by jteh on 2014-05-26 10:02
Great, except I think you forgot to change references to the config name in core.py. :)

Comment 10 by Joseph Lee <joseph.lee22590@... on 2014-05-26 10:18
In [e482861]:

cCore: apply changes to config variable name (caught by jteh). re #834.

Comment 11 by nvdakor (in reply to comment 9) on 2014-05-26 10:19
Replying to jteh:

Great, except I think you forgot to change references to the config name in core.py. :)

Thanks for catching that - fixed.

Comment 12 by James Teh <jamie@... on 2014-05-27 01:12
In [06fef97]:

Merge branch 't834' into next

Incubates #834.

Changes:
Added labels: incubating

Comment 13 by jteh on 2014-05-27 01:26
I found one more occurrence where the config name wasn't updated, which I fixed in 1341a8d. Obviously, this stuff is easy to miss, but it'd be great if you can double check in future to avoid delays and unnecessary code review iterations.

One thing that might help is to review the diff of all of your changes. To do this, you need the revision where you branched off master (the branch point); in this case, da88d67. Then, you do, for example:

git diff t834 da88d671

Btw, to get the branch point automatically, you can do, for example:

git rev-list master..t834 | tail -1

Changes:
Milestone changed from None to next

Comment 14 by James Teh <jamie@... on 2014-06-24 07:59
In [e953bd3]:

The sounds played when NVDA starts and exits can be disabled via a new option in the General Settings dialog.

Fixes #834.

Changes:
Removed labels: incubating
State: closed

Comment 15 by jteh on 2014-06-24 07:59
Changes:
Milestone changed from next to 2014.3

Comment 16 by dhankuta on 2014-06-27 07:05
Hi all,
Though the ticket has gone in close status ; I have made this feature with a dropdown choices instead the yes/no mode. It provides choices as follows:

1. on start and exit.
2. on start.
3. on exit.
4. never
   It is tested and is working well.
   If felt better than the already adopted Boolean option, ready to share the patch. Otherwise thanks.

Comment 17 by jteh on 2014-06-27 07:09
Is there a good case for separating these? In particular, this has already been merged to master, so any change to this setting will cause problems for existing users of master. This means we want to have a pretty solid justification for changing it. Thanks.