From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from re-prd-fep-048.btinternet.com (mailomta12-re.btinternet.com [213.120.69.105]) by sourceware.org (Postfix) with ESMTPS id 1820B3856DCB for ; Tue, 23 Aug 2022 15:16:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1820B3856DCB Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=dronecode.org.uk Authentication-Results: sourceware.org; spf=none smtp.mailfrom=dronecode.org.uk Received: from re-prd-rgout-005.btmx-prd.synchronoss.net ([10.2.54.8]) by re-prd-fep-048.btinternet.com with ESMTP id <20220823151653.MIKT3057.re-prd-fep-048.btinternet.com@re-prd-rgout-005.btmx-prd.synchronoss.net>; Tue, 23 Aug 2022 16:16:53 +0100 Authentication-Results: btinternet.com; auth=pass (PLAIN) smtp.auth=jonturney@btinternet.com; bimi=skipped X-SNCR-Rigid: 613A912435FC9B63 X-Originating-IP: [86.139.158.127] X-OWM-Source-IP: 86.139.158.127 (GB) X-OWM-Env-Sender: jonturney@btinternet.com X-VadeSecure-score: verdict=clean score=0/300, class=clean X-RazorGate-Vade: gggruggvucftvghtrhhoucdtuddrgedvfedrvdeiledgkeeiucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuueftkffvkffujffvgffngfevqffopdfqfgfvnecuuegrihhlohhuthemuceftddunecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenucfjughrpefkffggfgfuvfhfhfgjtgfgsehtkeertddtfeejnecuhfhrohhmpeflohhnucfvuhhrnhgvhicuoehjohhnrdhtuhhrnhgvhiesughrohhnvggtohguvgdrohhrghdruhhkqeenucggtffrrghtthgvrhhnpeeludejfefgvdfhkeejhfegieeuffetkeehhedufedtueetieeuveeugfejveehueenucffohhmrghinhepshhouhhrtggvfigrrhgvrdhorhhgnecukfhppeekiedrudefledrudehkedruddvjeenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhephhgvlhhopegludelvddrudeikedruddruddthegnpdhinhgvthepkeeirddufeelrdduheekrdduvdejpdhmrghilhhfrhhomhepjhhonhdrthhurhhnvgihsegurhhonhgvtghouggvrdhorhhgrdhukhdpnhgspghrtghpthhtohepvddprhgtphhtthhopeevhhhrihhsthhirghnrdfhrhgrnhhkvgesthdqohhnlhhinhgvrdguvgdprhgtphhtthhopegthihgfihinhdqrghpphhssegthihgfihinhdrtghomh X-RazorGate-Vade-Verdict: clean 0 X-RazorGate-Vade-Classification: clean Received: from [192.168.1.105] (86.139.158.127) by re-prd-rgout-005.btmx-prd.synchronoss.net (5.8.716.04) (authenticated as jonturney@btinternet.com) id 613A912435FC9B63; Tue, 23 Aug 2022 16:16:53 +0100 Message-ID: Date: Tue, 23 Aug 2022 16:16:52 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 Subject: Re: [PATCH setup] Keyboard accelerators for install/reinstall/uninstall Content-Language: en-GB To: Christian Franke , "cygwin-apps@cygwin.com" References: <28d2858f-a7b3-ba64-261c-c38e6bfa02db@t-online.de> <3b69d761-018e-dbc3-db57-369f6bf057bb@t-online.de> From: Jon Turney In-Reply-To: <3b69d761-018e-dbc3-db57-369f6bf057bb@t-online.de> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3567.4 required=5.0 tests=BAYES_00, BODY_8BITS, FORGED_SPF_HELO, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, NICE_REPLY_A, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, 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 15:16:57 -0000 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. >>> 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... >>> @@ -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? I forget how this all works, and there probably are separate paths which duplicate stuff, but this suggests to me that this is increasing that duplication, which isn't the desired direction of travel... > BTW, I didn't understand this line: > >   void >   packagemeta::set_action (...) >   { >     ... >     else if (action == Install_action) >       { >         desired = default_version; >         if (desired) >           { >             if (desired != installed) >               if (desired.accessible ()) >                 { >                   ... >                   pick (true); >                   srcpick (false); >                 } >               else >                 { >                   pick (false); >                   srcpick (true); <== why true? == >                 } >               ... > Yeah, I don't understand why that's there either. It seems like it's going to give you the source package if you've asked for the package to be installed, but it's not available. I think It's some ancient cruft which nobody would notice if we removed :)