From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from sa-prd-fep-049.btinternet.com (mailomta13-sa.btinternet.com [213.120.69.19]) by sourceware.org (Postfix) with ESMTPS id D4B593858D1E for ; Mon, 15 Aug 2022 17:49:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D4B593858D1E 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 sa-prd-rgout-002.btmx-prd.synchronoss.net ([10.2.38.5]) by sa-prd-fep-049.btinternet.com with ESMTP id <20220815174949.VQJW3227.sa-prd-fep-049.btinternet.com@sa-prd-rgout-002.btmx-prd.synchronoss.net>; Mon, 15 Aug 2022 18:49:49 +0100 Authentication-Results: btinternet.com; auth=pass (PLAIN) smtp.auth=jonturney@btinternet.com; bimi=skipped X-SNCR-Rigid: 6139417C359F602B X-Originating-IP: [81.153.98.171] X-OWM-Source-IP: 81.153.98.171 (GB) X-OWM-Env-Sender: jonturney@btinternet.com X-VadeSecure-score: verdict=clean score=0/300, class=clean X-RazorGate-Vade: gggruggvucftvghtrhhoucdtuddrgedvfedrvdehvddguddukecutefuodetggdotefrodftvfcurfhrohhfihhlvgemuceutffkvffkuffjvffgnffgvefqofdpqfgfvfenuceurghilhhouhhtmecufedtudenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujfgurhepkfffgggfuffvfhfhjggtgfesthejredttdefjeenucfhrhhomheplfhonhcuvfhurhhnvgihuceojhhonhdrthhurhhnvgihsegurhhonhgvtghouggvrdhorhhgrdhukheqnecuggftrfgrthhtvghrnhepiefhgefhvdeivdejveekkedtjeduvdehtdevteegveekgfdtieegjedvueethfdvnecuffhomhgrihhnpehsohhurhgtvgifrghrvgdrohhrghenucfkphepkedurdduheefrdelkedrudejudenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhephhgvlhhopegludelvddrudeikedruddruddthegnpdhinhgvthepkedurdduheefrdelkedrudejuddpmhgrihhlfhhrohhmpehjohhnrdhtuhhrnhgvhiesughrohhnvggtohguvgdrohhrghdruhhkpdhnsggprhgtphhtthhopedvpdhrtghpthhtohepvehhrhhishhtihgrnhdrhfhrrghnkhgvsehtqdhonhhlihhnvgdruggvpdhrtghpthhtoheptgihghifihhnqdgrphhpshestgihghifihhnrdgtohhm X-RazorGate-Vade-Verdict: clean 0 X-RazorGate-Vade-Classification: clean Received: from [192.168.1.105] (81.153.98.171) by sa-prd-rgout-002.btmx-prd.synchronoss.net (5.8.716.04) (authenticated as jonturney@btinternet.com) id 6139417C359F602B; Mon, 15 Aug 2022 18:49:49 +0100 Message-ID: Date: Mon, 15 Aug 2022 18:49:49 +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> From: Jon Turney In-Reply-To: <28d2858f-a7b3-ba64-261c-c38e6bfa02db@t-online.de> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-1197.9 required=5.0 tests=BAYES_00, FORGED_SPF_HELO, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham 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: Mon, 15 Aug 2022 17:49:54 -0000 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. > 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? > > 0001-Keyboard-accelerators-for-install-reinstall-uninstal.patch > > From 71da4fbc68022b5083eba0cbdf2c0a4a541ddf1c Mon Sep 17 00:00:00 2001 > From: Christian Franke > Date: Sun, 14 Aug 2022 13:43:48 +0200 > Subject: [PATCH] Keyboard accelerators for install/reinstall/uninstall > > Ctrl+I/R/U select install/reinstall/uninstall and then move selection > to next list item. > --- > ListView.cc | 17 +++++++++++++++-- > ListView.h | 3 ++- > PickCategoryLine.cc | 3 ++- > PickCategoryLine.h | 3 ++- > PickPackageLine.cc | 17 ++++++++++++++++- > PickPackageLine.h | 3 ++- > package_meta.cc | 4 ++++ > 7 files changed, 43 insertions(+), 7 deletions(-) > > diff --git a/ListView.cc b/ListView.cc > index 62a37ab..22009ba 100644 > --- a/ListView.cc > +++ b/ListView.cc > @@ -564,14 +564,20 @@ ListView::OnNotify (NMHDR *pNmHdr, LRESULT *pResult) > NMLVKEYDOWN *pNmLvKeyDown = (NMLVKEYDOWN *)pNmHdr; > int iRow = ListView_GetSelectionMark(hWndListView); > #if DEBUG > - Log (LOG_PLAIN) << "LVN_KEYDOWN vkey " << pNmLvKeyDown->wVKey << " on row " << iRow << endLog; > + Log (LOG_PLAIN) << "LVN_KEYDOWN vkey " << pNmLvKeyDown->wVKey << " on row " << iRow > + << " Ctrl:" << !!(GetKeyState(VK_CONTROL) & 0x8000) > + << " Alt:" << !!(GetKeyState(VK_MENU) & 0x8000) << endLog; > #endif > > if (contents && iRow >= 0) > { > + bool ctrl = !!(GetKeyState(VK_CONTROL) & 0x8000); > + bool alt = !!(GetKeyState(VK_MENU) & 0x8000); > int col_num; > int action_id; > - if ((*contents)[iRow]->map_key_to_action(pNmLvKeyDown->wVKey, &col_num, &action_id)) > + bool down = false; > + if ((*contents)[iRow]->map_key_to_action(pNmLvKeyDown->wVKey, ctrl, alt, &col_num, > + &action_id, &down)) > { > int update; > if (action_id >= 0) > @@ -591,6 +597,13 @@ ListView::OnNotify (NMHDR *pNmHdr, LRESULT *pResult) > if (update > 0) > ListView_RedrawItems(hWndListView, iRow, iRow + update -1); > } > + if (down && iRow + 1 < ListView_GetItemCount(hWndListView)) { Again as a stylistic thing, I'd suggest perhaps changing map_key_to_action() to return a set of flags which could include "ACTION", "POPUP" and "DOWN", rather than adding another flag parameter to it... > + ListView_SetItemState(hWndListView, -1, 0, LVIS_SELECTED | LVIS_FOCUSED); > + ListView_SetItemState(hWndListView, iRow + 1, LVIS_SELECTED | LVIS_FOCUSED, > + LVIS_SELECTED | LVIS_FOCUSED); > + ListView_SetSelectionMark(hWndListView, iRow + 1); > + ListView_EnsureVisible(hWndListView, iRow + 1, false); A comment here saying "and move selection to next row". > + } > } > } > break; > diff --git a/ListView.h b/ListView.h > index 95dd9ee..a2ecdef 100644 > --- a/ListView.h > +++ b/ListView.h > @@ -38,7 +38,8 @@ class ListViewLine > virtual ActionList *get_actions(int col) const = 0; > virtual int do_action(int col, int id) = 0; > virtual int do_default_action(int col) = 0; > - virtual bool map_key_to_action(WORD vkey, int *col_num, int *action_id) const = 0; > + virtual bool map_key_to_action(WORD vkey, bool ctrl, bool alt, int *col_num, > + int *action_id, bool *down) const = 0; > }; > > typedef std::vector ListViewContents; > diff --git a/PickCategoryLine.cc b/PickCategoryLine.cc > index d2ac899..9872a2e 100644 > --- a/PickCategoryLine.cc > +++ b/PickCategoryLine.cc > @@ -97,7 +97,8 @@ PickCategoryLine::get_tooltip(int col_num) const > } > > bool > -PickCategoryLine::map_key_to_action(WORD vkey, int *col_num, int *action_id) const > +PickCategoryLine::map_key_to_action(WORD vkey, bool ctrl, bool alt, int *col_num, > + int *action_id, bool *down) const > { > switch (vkey) > { > diff --git a/PickCategoryLine.h b/PickCategoryLine.h > index 6a7321d..0bfba33 100644 > --- a/PickCategoryLine.h > +++ b/PickCategoryLine.h > @@ -41,7 +41,8 @@ public: > ActionList *get_actions(int col) const; > int do_action(int col, int action_id); > int do_default_action(int col); > - bool map_key_to_action(WORD vkey, int *col_num, int *action_id) const; > + bool map_key_to_action(WORD vkey, bool ctrl, bool alt, int *col_num, > + int *action_id, bool *down) const; > > private: > CategoryTree * cat_tree; > diff --git a/PickPackageLine.cc b/PickPackageLine.cc > index ae1e520..ba47e1f 100644 > --- a/PickPackageLine.cc > +++ b/PickPackageLine.cc > @@ -145,7 +145,8 @@ PickPackageLine::get_indent() const > } > > bool > -PickPackageLine::map_key_to_action(WORD vkey, int *col_num, int *action_id) const > +PickPackageLine::map_key_to_action(WORD vkey, bool ctrl, bool alt, int *col_num, > + int *action_id, bool *down) const > { > switch (vkey) > { > @@ -154,6 +155,20 @@ PickPackageLine::map_key_to_action(WORD vkey, int *col_num, int *action_id) cons > *col_num = new_col; > *action_id = -1; > return true; > + case 'I': > + case 'R': > + case 'U': > + if (!(ctrl && !alt)) As a stylistic thing, I'd perhaps rather combine ctrl and alt flags as a modifier bitmap. What is the reasoning for selecting the "ctrl but not alt" modifier state here? > + break; > + *col_num = new_col; > + switch (vkey) > + { > + case 'I': *action_id = packagemeta::Install_action; break; > + case 'R': *action_id = packagemeta::Reinstall_action; break; > + default: *action_id = packagemeta::Uninstall_action; break; > + } > + *down = true; > + return true; > } > > return false; > diff --git a/PickPackageLine.h b/PickPackageLine.h > index 2c59e90..9eb09db 100644 > --- a/PickPackageLine.h > +++ b/PickPackageLine.h > @@ -37,7 +37,8 @@ public: > ActionList *get_actions(int col_num) const; > int do_action(int col, int action_id); > int do_default_action(int col); > - bool map_key_to_action(WORD vkey, int *col_num, int *action_id) const; > + bool map_key_to_action(WORD vkey, bool ctrl, bool alt, int *col_num, > + int *action_id, bool *down) const; > private: > packagemeta & pkg; > PickView & theView; > diff --git a/package_meta.cc b/package_meta.cc > index 8a69525..05b8946 100644 > --- a/package_meta.cc > +++ b/package_meta.cc > @@ -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? > } > > _action = action;