[PATCH setup] Keyboard accelerators for install/reinstall/uninstall
Jon Turney
jon.turney@dronecode.org.uk
Fri Aug 26 13:33:14 GMT 2022
On 23/08/2022 17:44, Christian Franke wrote:
> Jon Turney wrote:
>> On 22/08/2022 16:29, Christian Franke wrote:
>>> Jon Turney wrote:
>>>> On 14/08/2022 12:57, Christian Franke wrote:
>>>>> This eases state changes of a selected sequence of packages.
>>>>
>>>> Nice! The keyboard control of the package chooser was a bit of an
>>>> after-thought, which it really shouldn't be.
>>>
>>> Thanks - revised patch is attached.
>>>
>>
>> Thanks. This looks good. Please apply.
>
> Thanks. Applied.
>
>
>>
>>>>> Ctrl+U is in particular useful to cleanup installations in
>>>>> conjunction with "unneeded" view:
>>>>> https://sourceware.org/pipermail/cygwin-apps/2022-August/042185.html
>>>>>
>>>>> Open issue: Add some visual clue (tooltip?) to make this
>>>>> functionality more obvious.
>>>>
>>>> Yeah. These shortcuts should also be accelerators for the package
>>>> action selection popup menu, which would make them more discoverable?
>>>
>>> Handling these in the popup menu is possibly tricky. According to
>>> documentation of TrackPopupMenu(), hWndListView "receives all
>>> messages from the menu" which is apparently not the case.
>>
>> Confused. I don't think that matters (we could probably be using
>> TPM_NONOTIFY), because we use TPM_RETURNCMD.
>>
>> If we mark accelerators in the menu:
>>
>> --- a/res/en/res.rc
>> +++ b/res/en/res.rc
>> @@ -573,11 +573,11 @@ BEGIN
>> IDS_PROGRESS_POSTINSTALL "Running..."
>> IDS_PROGRESS_SOLVING "Solving dependencies..."
>> IDS_ACTION_DEFAULT "Default"
>> - IDS_ACTION_INSTALL "Install"
>> - IDS_ACTION_UNINSTALL "Uninstall"
>> + IDS_ACTION_INSTALL "&Install"
>> + IDS_ACTION_UNINSTALL "&Uninstall"
>> IDS_ACTION_SKIP "Skip"
>> IDS_ACTION_KEEP "Keep"
>> - IDS_ACTION_REINSTALL "Reinstall"
>> + IDS_ACTION_REINSTALL "&Reinstall"
>> IDS_ACTION_RETRIEVE "Retrieve"
>> IDS_ACTION_UNKNOWN "Unknown"
>> IDS_ACTION_SOURCE "Source"
>>
>> They appear when the menu is opened by pressing the menu key (or
>> always, if "Underline access keys when available" is on in
>> ease-of-access settings), and menu items can be chosen using them.
>>
>> It's not quite that straightforward because we need to remove the '&'
>> when those strings are used elsewhere (e.g. in the action column), but
>> I think it can be done...
>
> Using plain letters (without Ctrl) or first digit of version number
> already work in the popup menu. But it is possibly tricky to also
> interpret Ctrl+I/R/U in the popup menu.
>
I think we might be talking at cross-purposes here.
I don't think we need to do anything specific to make accelerators work
in that menu, because TrackMenuPopup() is running it's own modal message
loop which does TranslateAccelerator() for it...
>>>>> @@ -670,6 +670,10 @@ packagemeta::set_action (_actions action,
>>>>> packageversion const &default_version,
>>>>> else if (action == Uninstall_action)
>>>>> {
>>>>> desired = packageversion ();
>>>>> + pick (false);
>>>>> + srcpick (false);
>>>>> + if (!installed)
>>>>> + action = NoChange_action;
>>>>
>>>> Hmm... why is adding this needed?
>>>
>>> Otherwise a strange state change would occur at least in the GUI when
>>> an install request is undone:
>>>
>>> "Skip" == Ctrl+I ==> "3.2-1" == Ctrl+U ==> "Uninstall"
>>>
>>> The new patch includes another addition which prevents this on
>>> installs from local directory when the current default version is not
>>> yet downloaded:
>>>
>>> "Skip" == Ctrl+I ==> "" (empty)
>>
>> I see. But these work correctly when chosen via the action menu
>> dropdown?
>
> These state changes are not offered by the popup menu. For example for
> not installed packages the popup menu does not contain a selection which
> returns Uninstall_action. Undoing an install request is done via
> re-selection of "Skip" which returns NoChange_action.
>
> The design alternatives were either to emulate this in
> PickPackageLine::map_key_to_action() or to complete the (hidden) state
> machine in packagemeta::set_action(). I decided to use the latter.
Ok, that makes perfect sense. Thanks!
More information about the Cygwin-apps
mailing list