[PATCH setup] Add tooltip for Ctrl+I/R/U accelerator keys

[PATCH setup] Add tooltip for Ctrl+I/R/U accelerator keys

[Christian Franke]

[-- Attachment #1: Type: text/plain, Size: 224 bytes --]

This is a first try to make these keys more obvious as requested on the 
Cygwin ML.

A more complex approach would be to mention only the keys which actually 
would change the current package state.

-- 
Regards,
Christian


[-- Attachment #2: 0001-Add-tooltip-for-Ctrl-I-R-U-accelerator-keys.patch --]
[-- Type: text/plain, Size: 4863 bytes --]

From 6bfb1f07c772fd6298c63b3b2ae53000e26a6237 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Tue, 22 Nov 2022 16:38:39 +0100
Subject: [PATCH] Add tooltip for Ctrl+I/R/U accelerator keys

Tooltip is shown for "New" column of selected row.
---
 ListView.cc         | 3 ++-
 ListView.h          | 2 +-
 PickCategoryLine.cc | 2 +-
 PickCategoryLine.h  | 2 +-
 PickPackageLine.cc  | 9 +++++++--
 PickPackageLine.h   | 2 +-
 res/en/res.rc       | 1 +
 res/fr/res.rc       | 1 +
 resource.h          | 1 +
 9 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/ListView.cc b/ListView.cc
index 7cc7f0f..40addaa 100644
--- a/ListView.cc
+++ b/ListView.cc
@@ -643,7 +643,8 @@ ListView::OnNotify (NMHDR *pNmHdr, LRESULT *pResult)
       static StringCache<std::string> tooltip;
       tooltip = "";
       if (contents)
-        tooltip = (*contents)[iRow]->get_tooltip(iCol);
+        tooltip = (*contents)[iRow]->get_tooltip(iCol, GetFocus() == hWndListView
+                  && ListView_GetSelectionMark(hWndListView) == iRow);
 
       // set the tooltip text
       NMTTDISPINFO *pNmTTDispInfo = (NMTTDISPINFO *)pNmHdr;
diff --git a/ListView.h b/ListView.h
index 6a1be0b..2703d1f 100644
--- a/ListView.h
+++ b/ListView.h
@@ -40,7 +40,7 @@ class ListViewLine
   virtual ~ListViewLine() {};
   virtual const std::wstring get_text(int col) const = 0;
   virtual State get_state() const = 0;
-  virtual const std::string get_tooltip(int col) const = 0;
+  virtual const std::string get_tooltip(int col, bool selected) const = 0;
   virtual int get_indent() const = 0;
   virtual ActionList *get_actions(int col) const = 0;
   virtual int do_action(int col, int id) = 0;
diff --git a/PickCategoryLine.cc b/PickCategoryLine.cc
index b13dbe4..d9f9535 100644
--- a/PickCategoryLine.cc
+++ b/PickCategoryLine.cc
@@ -91,7 +91,7 @@ PickCategoryLine::get_indent() const
 }
 
 const std::string
-PickCategoryLine::get_tooltip(int col_num) const
+PickCategoryLine::get_tooltip(int col_num, bool selected) const
 {
   return "";
 }
diff --git a/PickCategoryLine.h b/PickCategoryLine.h
index 7616b15..8352c36 100644
--- a/PickCategoryLine.h
+++ b/PickCategoryLine.h
@@ -36,7 +36,7 @@ public:
 
   const std::wstring get_text(int col) const;
   State get_state() const;
-  const std::string get_tooltip(int col) const;
+  const std::string get_tooltip(int col, bool selected) const;
   int get_indent() const;
   ActionList *get_actions(int col) const;
   int do_action(int col, int action_id);
diff --git a/PickPackageLine.cc b/PickPackageLine.cc
index c1e2a15..fd033e2 100644
--- a/PickPackageLine.cc
+++ b/PickPackageLine.cc
@@ -16,6 +16,7 @@
 #include "PickPackageLine.h"
 #include "PickView.h"
 #include "package_db.h"
+#include "resource.h"
 
 const std::wstring
 PickPackageLine::get_text(int col_num) const
@@ -86,9 +87,13 @@ PickPackageLine::get_text(int col_num) const
 }
 
 const std::string
-PickPackageLine::get_tooltip(int col_num) const
+PickPackageLine::get_tooltip(int col_num, bool selected) const
 {
-  if (col_num == pkg_col)
+  if (col_num == new_col && selected)
+    {
+      return wstring_to_string(LoadStringW(IDS_CTRL_IRU_TOOLTIP));
+    }
+  else if (col_num == pkg_col)
     {
       return pkg.LDesc();
     }
diff --git a/PickPackageLine.h b/PickPackageLine.h
index 0bf4ae6..f882ac8 100644
--- a/PickPackageLine.h
+++ b/PickPackageLine.h
@@ -32,7 +32,7 @@ public:
   };
   const std::wstring get_text(int col) const;
   State get_state() const { return State::nothing; }
-  const std::string get_tooltip(int col) const;
+  const std::string get_tooltip(int col, bool selected) const;
   int get_indent() const;
   ActionList *get_actions(int col_num) const;
   int do_action(int col, int action_id);
diff --git a/res/en/res.rc b/res/en/res.rc
index ef5e8b1..9b3288d 100644
--- a/res/en/res.rc
+++ b/res/en/res.rc
@@ -653,6 +653,7 @@ BEGIN
     IDS_CONFIRM_SOURCE "(source)"
     IDS_FILE_INUSE_KILL "&Kill Processes"
     IDS_FILE_INUSE_MSG "Unable to extract"
+    IDS_CTRL_IRU_TOOLTIP "Ctrl+I: Install, Ctrl+R: Reinstall, Ctrl+U: Uninstall"
 END
 
 //
diff --git a/res/fr/res.rc b/res/fr/res.rc
index 747e1dd..b6d2f6d 100644
--- a/res/fr/res.rc
+++ b/res/fr/res.rc
@@ -633,6 +633,7 @@ BEGIN
     IDS_CONFIRM_SOURCE "(source)"
     IDS_FILE_INUSE_KILL "&Tuer les processus"
     IDS_FILE_INUSE_MSG "Incapable d'extraire"
+    // IDS_CTRL_IRU_TOOLTIP "XXX: missing translation"
 END
 
 //
diff --git a/resource.h b/resource.h
index cfe860b..4b636d2 100644
--- a/resource.h
+++ b/resource.h
@@ -107,6 +107,7 @@
 #define IDS_DEPRECATED_WINDOWS_ARCH      1210
 #define IDS_VIEW_REMOVABLE               1211
 #define IDS_VIEW_UNNEEDED                1212
+#define IDS_CTRL_IRU_TOOLTIP             1213
 
 #define IDS_HELPTEXT_COMPACTOS           1500
 #define IDS_HELPTEXT_PUBKEY              1501
-- 
2.38.1

Re: [PATCH setup] Add tooltip for Ctrl+I/R/U accelerator keys

[Jon Turney]

On 22/11/2022 16:00, Christian Franke wrote:
> This is a first try to make these keys more obvious as requested on the 
> Cygwin ML.
> 
> A more complex approach would be to mention only the keys which actually 
> would change the current package state.

Thanks for this patch.

So, I appreciate what this is trying to do, but I have great difficultly 
convincing myself that this is the right approach.

We don't need to resort to a tooltip for any other shortcut, because 
there is an established visual design language to indicate those.  I 
guess we just need to find what's consistent with pre-existing ways of 
indicating this kind of behaviour in other applications.

Re: [PATCH setup] Add tooltip for Ctrl+I/R/U accelerator keys

[Christian Franke]

Jon Turney wrote:
> On 22/11/2022 16:00, Christian Franke wrote:
>> This is a first try to make these keys more obvious as requested on 
>> the Cygwin ML.
>>
>> A more complex approach would be to mention only the keys which 
>> actually would change the current package state.
>
> Thanks for this patch.
>
> So, I appreciate what this is trying to do, but I have great 
> difficultly convincing myself that this is the right approach.
>

I'm also not sure. An alternative would possibly be some help below the 
listbox, centered in the same line as "Hide obsolete packages".


> We don't need to resort to a tooltip for any other shortcut, because 
> there is an established visual design language to indicate those.  I 
> guess we just need to find what's consistent with pre-existing ways of 
> indicating this kind of behaviour in other applications.
>

Cygwin setup is somewhat uncommon as there is no default menu bar with 
drop-down menus. Accelerator keys are traditionally mentioned there.