Add-ons manager: It is now possible to disable individual add-ons. re #3090#6079
Merged
michaelDCurran merged 24 commits intonvaccess:masterfrom Jul 24, 2016
Merged
Add-ons manager: It is now possible to disable individual add-ons. re #3090#6079michaelDCurran merged 24 commits intonvaccess:masterfrom
michaelDCurran merged 24 commits intonvaccess:masterfrom
Conversation
…vaccess#3090. In order to allow users to disable add-ons from add-ons manager or from command line, it is necessary to tweak the add-ons subsystem a bit. Specifically: * Core.py: If all add-ons are disabled, this fact is recorded in the log, useful for troubleshooting purposes. * addonHandler.py: Added a new pending disable set so add-ons can be disabled upon startup. The user interface to manipulate this set will be committed later. Also added a function (disableAddonsIfAny) that'll save current disable state to a private set (can be extended to supporting adding add-ons from command line). * addonHandler.Addon: Added isDisabled property to report if the currently selected add-on is disabled. Also modified isRunning attribute to check if the add-on is disabled. These properties will come in handy when presenting state information in add-ons manager. So far, tests show it is working. Next on the list is adding few more properties such as pending enable/disable to addonHandler.Addon, then start implementing the user interface.
…ns upon request (part 1). re nvaccess#3090. Added a routine in addonHandler.Addon to enable or disable add-ons upon request, set via a new button in add-ons manager (no hotkey assigned to prevent users from accidentally disabling wrong add-ons). The next commit will be display strings for when an add-on is enable and disabled.
…cted add-on. re nvaccess#3090. Suggested by Norlia Martinez: prompt for confirmation before disabling the selected add-on to make sure users know what they are doing. Also caught KeyError exception, likely to be encountered when installing a build which includes this feature for the first time.
…hown, documented add-on enable/disable feature. re nvaccess#3090. Although it was suggested to use a dedicated set for pending enables, it is better to use a module-level set, as pendingDisableSet is really intended to tell disabled add-ons set what to disable. Also, because isDisabled flag takes precedence, enable/disalbe status is fetched early. Also documented add-on enable/disable feature in the user guide. As of this commit, all the pieces for enabling/disabling add-ons and its user interface are there (next up is command-line version, a bit tricky but doable).
…, as it is not feasible. re nvaccess#3090
…rt screen. re nvaccess#3090 Good catch by Derek Riemer: always strive for consistency in user interface strings.
source/gui/addonGui.py
Outdated
| self.removeButton.Disable() | ||
|
|
||
| def _shouldDisable(self, addon): | ||
| return not (addon.isPendingDisable or (addon.isDisabled and not addon.isPendingEnable and not addon.isPendingDisable)) |
Member
There was a problem hiding this comment.
This is a confusing bit of logic. Do you need to check addon.isPendingDisable twice? I would have thought that if it got to the inner parenthesis, isPendingDisable could only be false?
Member
|
A case which is a bit strange is where you disable an add-on, don't restart, and then re-enable the add-on. This causes the status to change to "enable" and a restart of NVDA is still required. Really, the status should go back to "running" and a restart should not be necessary. I think the easiest way is to change needsRestart into a property which checks to see if any of the "pending" properties are true. Instead of it being set by each action explicitly. Then when enabling an add-on who is pending a disable, the pending disable can be turned off and the status set back to "running". |
…ling add-ons, simplify restart prompt. re nvaccess#3090. Code review from Mick Curran (NV Access): * Restart prompt: quite complicated (agreed). Thus simplify to say that changes were made to add-ons. * Disable warning: Although it is good to give users warnings similar to the one shown when installing an add-on, it would be better to just disable or enable it and be done with it. Thus no warning from now on. * User guide: linguistic fixes.
…t a restart. re nvaccess#3090 Consider the following scenario: user opens add-ons manager, disables and add-on, then enables it without a restart. Status columns should say 'running' as opposed to 'enable' (the current one is redundant, as enabling means running). Caught by Mick Curran (NV Access): NvDA will just say 'runing' for this case.
source/gui/addonGui.py
Outdated
| self.helpButton.Bind(wx.EVT_BUTTON,self.onHelp) | ||
| entryButtonsSizer.Add(self.helpButton) | ||
| # Translators: The label for a button in Add-ons Manager dialog to enable or disable the selected add-on. | ||
| self.toggleButton=wx.Button(self,label=_("Disable add-on")) |
Member
There was a problem hiding this comment.
Rename the toggleButton variable to enableDisableButton or something similar. ToggleButton I feel is not clear enough...
Contributor
Author
|
Hi, Regarding the logic on should disable function: It is much better than saying (I think): Basically, there are three cases where this is true:
The structure of the truth table is:
Hope this helps. Thanks. |
Caught by Mick Curran: testing shows second check of addon.pendingDisable is actually unnecessary (besides being a performance issue later), hence pendingDisable set wilol be consulted only once.
… for cardinality of sets and comparing sets. re nvaccess#3090. As noted by Michael Curran: it is better to make needsRestart a property. However given that it is consulted only when closing, it is better to refactor by: * Check if any add-ons have been installed or removed ( tehnically, pending). * Check if disabled add-ons and pending disable set are the same (for the most part, it is). This also fixes an issue where if restart dialog is dismissed when closing add-ons manager, and add-ons manager is opened again, restart prompt would not be displayed.
…ler.terminate, rewrote needsRestart. re nvaccess#3090 Comments from Tyler Spivey and Michael Curran: when disabled, an add-on should stay disabled until enabled later. To hold this informaiton, addonHandler.termiante is now fleshed out with various set operations (intersection nad set difference mostly) to let NVDA know which add-ons should stay deactivated. Also rewrote needsRestart so it'll go through add-ons and raise the flag if the following conditions are seen: pending install, pending remove, about to come back to life (enabled), and about to stay ou4t of business (disabled).
user_docs/en/userGuide.t2t
Outdated
| To disable an add-on, press the disable button. | ||
| To enable a previously disabled add-on, press the enable button. | ||
| You can disable an add-on if the add-on status indicates it is running or enabled, or enable it if the add-on is suspended or disabled. | ||
| For each press of enable/disable button, add-on status changes to indicate what will happen when NVDA restarts. |
Member
There was a problem hiding this comment.
Needs a "the": "each press of [the] enable/disable button,"
Caught by Michael Curran: add 'the' to make it grammatically correct.
…hat we have a persistent backup in case of disasters such as crashes. re nvaccess#3090. A useful that wasn't thought of before (outlined by Michael Curran): what if NVDA suddenly crashes? If this happens, the previous states map would not record add-ons that are indeed disabled because pendingDisableSet is gone. Thus two new sets are introduced: * DisabledAddons: records add-ons that are actually disabled. * pendingEnableSet: Tells NVDA to enable the listed add-ons next time it is run. Due to this change: * pendingDisableSet will no longer record add-ons that'll be disabled during the next session. This set will now hold a list of add-ons that'll be turned off next time it loads. * Runtime counterparts of these sets will be kept in memory. When a crash occurs, persistent copy will be loaded.
…ntime states denoting future enable/disable. re nvaccess#3090
…s pending enable. re nvaccess#3090
…ed and is pending enable. re nvaccess#3090" This reverts commit f4e3852.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It is now possible to disable individual add-ons from add-ons manager. To support this, add-on handler has been modified as follows: