From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout01.t-online.de (mailout01.t-online.de [194.25.134.80]) by sourceware.org (Postfix) with ESMTPS id 7B7EF385AC1B for ; Tue, 23 Aug 2022 16:44:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7B7EF385AC1B Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=t-online.de Authentication-Results: sourceware.org; spf=none smtp.mailfrom=t-online.de Received: from fwd88.dcpf.telekom.de (fwd88.aul.t-online.de [10.223.144.114]) by mailout01.t-online.de (Postfix) with SMTP id 2AED9931D; Tue, 23 Aug 2022 18:44:53 +0200 (CEST) Received: from [192.168.2.101] ([79.230.170.147]) by fwd88.t-online.de with (TLSv1.3:TLS_AES_256_GCM_SHA384 encrypted) esmtp id 1oQX1F-04ynvE0; Tue, 23 Aug 2022 18:44:49 +0200 Subject: Re: [PATCH setup] Keyboard accelerators for install/reinstall/uninstall To: Jon Turney , "cygwin-apps@cygwin.com" References: <28d2858f-a7b3-ba64-261c-c38e6bfa02db@t-online.de> <3b69d761-018e-dbc3-db57-369f6bf057bb@t-online.de> From: Christian Franke Message-ID: Date: Tue, 23 Aug 2022 18:44:48 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 SeaMonkey/2.53.12 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-TOI-EXPURGATEID: 150726::1661273089-0144075D-B04A219E/0/0 CLEAN NORMAL X-TOI-MSGID: 5d778889-b714-40b3-8b3b-31bc8be2ad1b X-Spam-Status: No, score=-2.3 required=5.0 tests=BAYES_00, BODY_8BITS, FREEMAIL_FROM, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: cygwin-apps@cygwin.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Cygwin package maintainer discussion list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Aug 2022 16:44:57 -0000 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. > >>>> @@ -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.