[PATCH setup libsolv, v2] Let the user review added dependencies

[PATCH setup libsolv, v2] Let the user review added dependencies

[Ken Brown]

If the solver found no problems but added packages to resolve
dependencies, ask the user whether they want to review the added
packages before proceeding.

If they answer Yes, go back to the chooser with the 'Pending' view
selected.  The implementation adds several new members to the
PrereqChecker class so that the latter can communicate with the
chooser page.
---
 choose.cc |  3 +++
 prereq.cc | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
 prereq.h  |  8 ++++++++
 3 files changed, 70 insertions(+), 9 deletions(-)

diff --git a/choose.cc b/choose.cc
index 1c79cca..846b169 100644
--- a/choose.cc
+++ b/choose.cc
@@ -286,6 +286,9 @@ ChooserPage::OnInit ()
   /* Set focus to search edittext control. */
   PostMessage (GetHWND (), WM_NEXTDLGCTL,
 	       (WPARAM) GetDlgItem (IDC_CHOOSE_SEARCH_EDIT), TRUE);
+
+  /* Store the HWNDs that the PrereqChecker will need.  */
+  PrereqChecker::setChooserHWNDs (GetHWND (), viewlist);
 }
 
 void
diff --git a/prereq.cc b/prereq.cc
index effa7cc..7821d63 100644
--- a/prereq.cc
+++ b/prereq.cc
@@ -35,6 +35,7 @@
 #include "msg.h"
 #include "Exception.h"
 #include "getopt++/BoolOption.h"
+#include "PickView.h"
 
 // Sizing information.
 static ControlAdjuster::ControlInfo PrereqControlsInfo[] = {
@@ -139,8 +140,8 @@ PrereqPage::whatNext ()
   return IDD_INSTATUS;
 }
 
-long
-PrereqPage::OnBack ()
+static void
+prepBack ()
 {
   // Add reinstall tasks
   PrereqChecker p;
@@ -149,7 +150,12 @@ PrereqPage::OnBack ()
   // Reset the package database to correspond to the solver's solution
   packagedb db;
   db.solution.trans2db();
+}
 
+long
+PrereqPage::OnBack ()
+{
+  prepBack ();
   return IDD_CHOOSE;
 }
 
@@ -170,6 +176,7 @@ PrereqPage::OnUnattended ()
 // instantiate the static members
 bool PrereqChecker::use_test_packages;
 SolverTasks PrereqChecker::q;
+HWND PrereqChecker::hChooser, PrereqChecker::hChooseView;
 
 bool
 PrereqChecker::isMet ()
@@ -204,6 +211,15 @@ PrereqChecker::augment ()
   db.solution.augmentTasks(q);
 }
 
+void
+PrereqChecker::setChooserView (PickView::views view)
+{
+  PostMessage (hChooseView, CB_SETCURSEL, (WPARAM)view, 0);
+  PostMessage (hChooser, WM_COMMAND,
+	       MAKEWPARAM (IDC_CHOOSE_VIEW, CBN_SELCHANGE),
+	       (LPARAM)hChooseView);
+}
+
 /* Formats problems and solutions as a string for display to the user.  */
 void
 PrereqChecker::getUnmetString (std::string &s)
@@ -224,6 +240,23 @@ PrereqChecker::getUnmetString (std::string &s)
 // progress page glue
 // ---------------------------------------------------------------------------
 
+static bool
+added_deps ()
+{
+  packagedb db;
+  const SolverTransactionList & trans = db.solution.transactions ();
+  for (SolverTransactionList::const_iterator i = trans.begin ();
+       i != trans.end (); i++)
+    if (i->type == SolverTransaction::transInstall)
+      {
+	packageversion pv = i->version;
+	packagemeta *pkg = db.findBinary (PackageSpecification (pv.Name ()));
+	if (!pkg->desired)
+	  return true;
+      }
+  return false;
+}
+
 static int
 do_prereq_check_thread(HINSTANCE h, HWND owner)
 {
@@ -232,17 +265,34 @@ do_prereq_check_thread(HINSTANCE h, HWND owner)
 
   if (p.isMet ())
     {
-      p.finalize();
-
-      if (source == IDC_SOURCE_LOCALDIR)
-	Progress.SetActivateTask (WM_APP_START_INSTALL);  // install
+      if (added_deps () && !unattended_mode
+	  && MessageBox (owner, "Packages were added to resolve dependencies.  "
+			 "Do you want to review them before proceeding?"
+			 "\r\n\r\n"
+			 "Answering 'Yes' will take you back to the Package "
+			 "Selection page, with the 'Pending' view selected.",
+			 "Added Dependencies",
+			 MB_YESNO | MB_DEFBUTTON2) == IDYES)
+	{
+	  Progress.SetText1 ("Preparing package review...");
+	  prepBack ();
+	  p.setChooserView ();
+	  retval = IDD_CHOOSE;
+	}
       else
-	Progress.SetActivateTask (WM_APP_START_DOWNLOAD); // start download
-      retval = IDD_INSTATUS;
+	{
+	  p.finalize();
+
+	  if (source == IDC_SOURCE_LOCALDIR)
+	    Progress.SetActivateTask (WM_APP_START_INSTALL);  // install
+	  else
+	    Progress.SetActivateTask (WM_APP_START_DOWNLOAD); // start download
+	  retval = IDD_INSTATUS;
+	}
     }
   else
     {
-      // rut-roh, some required things are not selected
+      // The solver reported problems.
       retval = IDD_PREREQ;
     }
 
diff --git a/prereq.h b/prereq.h
index 24e6de5..d60ef59 100644
--- a/prereq.h
+++ b/prereq.h
@@ -5,6 +5,8 @@
 #include "proppage.h"
 #include "PackageTrust.h"
 #include "package_meta.h"
+#include "win32.h"
+#include "PickView.h"
 
 using namespace std;
 
@@ -45,11 +47,17 @@ public:
 
   void augment ();
 
+  void setChooserView (PickView::views = PickView::views::PackagePending);
+
   static void setTestPackages (bool t) { use_test_packages = t; };
 
+  static void setChooserHWNDs (HWND hc, HWND hv)
+  { hChooser = hc; hChooseView = hv; };
+
 private:
   static bool use_test_packages;
   static SolverTasks q;
+  static HWND hChooser, hChooseView;
 };
 
 #endif /* SETUP_PREREQ_H */
-- 
2.15.1

Re: [PATCH setup libsolv, v2] Let the user review added dependencies

[Brian Inglis]

On 2018-01-22 10:37, Ken Brown wrote:
> If the solver found no problems but added packages to resolve
> dependencies, ask the user whether they want to review the added
> packages before proceeding.
> 
> If they answer Yes, go back to the chooser with the 'Pending' view
> selected.  The implementation adds several new members to the
> PrereqChecker class so that the latter can communicate with the
> chooser page.

Would it not be better, quicker, and easier, and to just show the updated
Pending view directly and unconditionally, without another popup dialogue box?
I think that's what most of us would expect from a UI display: just update the
displayed packages, if not being asked for confirmation that we really want to
do something possibly detrimental.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

Re: [PATCH setup libsolv, v2] Let the user review added dependencies

[Ken Brown]

On 1/22/2018 4:51 PM, Brian Inglis wrote:
> On 2018-01-22 10:37, Ken Brown wrote:
>> If the solver found no problems but added packages to resolve
>> dependencies, ask the user whether they want to review the added
>> packages before proceeding.
>>
>> If they answer Yes, go back to the chooser with the 'Pending' view
>> selected.  The implementation adds several new members to the
>> PrereqChecker class so that the latter can communicate with the
>> chooser page.
> 
> Would it not be better, quicker, and easier, and to just show the updated
> Pending view directly and unconditionally, without another popup dialogue box?
> I think that's what most of us would expect from a UI display: just update the
> displayed packages, if not being asked for confirmation that we really want to
> do something possibly detrimental.

Yes, I think you're right, in principle.  But I'm not sure exactly what 
the UI should do.  If we simply update the displayed packages when the 
user selects 'Next', won't the user wonder what's going on?  Can you 
suggest a way to make it clear that we've updated the display to reflect 
added dependencies and that we're waiting for final confirmation?

Ken

Re: [PATCH setup libsolv, v2] Let the user review added dependencies

[Brian Inglis]

On 2018-01-22 15:40, Ken Brown wrote:
> On 1/22/2018 4:51 PM, Brian Inglis wrote:
>> On 2018-01-22 10:37, Ken Brown wrote:
>>> If the solver found no problems but added packages to resolve
>>> dependencies, ask the user whether they want to review the added
>>> packages before proceeding.
>>>
>>> If they answer Yes, go back to the chooser with the 'Pending' view
>>> selected.  The implementation adds several new members to the
>>> PrereqChecker class so that the latter can communicate with the
>>> chooser page.
>>
>> Would it not be better, quicker, and easier, and to just show the updated
>> Pending view directly and unconditionally, without another popup dialogue box?
>> I think that's what most of us would expect from a UI display: just update the
>> displayed packages, if not being asked for confirmation that we really want to
>> do something possibly detrimental.
> 
> Yes, I think you're right, in principle.  But I'm not sure exactly what the UI
> should do.  If we simply update the displayed packages when the user selects
> 'Next', won't the user wonder what's going on?  Can you suggest a way to make it
> clear that we've updated the display to reflect added dependencies and that
> we're waiting for final confirmation?

I see what you mean: we don't have a status line or any way to highlight added
dependencies which we could use to convey that, so unless the addition of
dependencies to the Pending view is visually very obvious, we need to use a
dialogue box to pass on the message, before proceeding from the Pending view.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

Re: [PATCH setup libsolv, v2] Let the user review added dependencies

[cyg Simple]

On 1/22/2018 8:07 PM, Brian Inglis wrote:
> On 2018-01-22 15:40, Ken Brown wrote:
>> On 1/22/2018 4:51 PM, Brian Inglis wrote:
>>> On 2018-01-22 10:37, Ken Brown wrote:
>>>> If the solver found no problems but added packages to resolve
>>>> dependencies, ask the user whether they want to review the added
>>>> packages before proceeding.
>>>>
>>>> If they answer Yes, go back to the chooser with the 'Pending' view
>>>> selected.  The implementation adds several new members to the
>>>> PrereqChecker class so that the latter can communicate with the
>>>> chooser page.
>>>
>>> Would it not be better, quicker, and easier, and to just show the updated
>>> Pending view directly and unconditionally, without another popup dialogue box?
>>> I think that's what most of us would expect from a UI display: just update the
>>> displayed packages, if not being asked for confirmation that we really want to
>>> do something possibly detrimental.
>>
>> Yes, I think you're right, in principle.  But I'm not sure exactly what the UI
>> should do.  If we simply update the displayed packages when the user selects
>> 'Next', won't the user wonder what's going on?  Can you suggest a way to make it
>> clear that we've updated the display to reflect added dependencies and that
>> we're waiting for final confirmation?
> 
> I see what you mean: we don't have a status line or any way to highlight added
> dependencies which we could use to convey that, so unless the addition of
> dependencies to the Pending view is visually very obvious, we need to use a
> dialogue box to pass on the message, before proceeding from the Pending view.
> 

How about changing the background color when the pending view list
changes?  The typical case for added dependencies is to just accept
them; especially if the primary package is really wanted.  Maybe rotate
between three or four colors to indicate new data in the list for each
selection but just two is good as well.

Also maybe changing the text on the Next button to indicate the total
number of packages pending action.  It would be another visual clue.

-- 
cyg Simple

Re: [PATCH setup libsolv, v2] Let the user review added dependencies

[Jon Turney]

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

On 22/01/2018 17:37, Ken Brown wrote:
> If the solver found no problems but added packages to resolve
> dependencies, ask the user whether they want to review the added
> packages before proceeding.
> 
> If they answer Yes, go back to the chooser with the 'Pending' view
> selected.  The implementation adds several new members to the
> PrereqChecker class so that the latter can communicate with the
> chooser page.

As discussed, this approach could be confusing.

Attached is a slightly different approach, which adds a new page to 
review and confirm what actions we're going to take.

For the moment, this just contains a simple text report, but I guess 
this could be extended e.g. to use a grid control, or give reasons for 
why packages are being installed.

[-- Attachment #2: 0001-Add-a-new-page-to-let-the-user-review-and-confirm-ac.patch --]
[-- Type: text/plain, Size: 10366 bytes --]

From f73030816b16d9dbcb0ed13ce84ee6c7914d8c3e Mon Sep 17 00:00:00 2001
From: Jon Turney <jon.turney@dronecode.org.uk>
Date: Tue, 23 Jan 2018 17:50:13 +0000
Subject: [PATCH setup] Add a new page to let the user review and confirm
 actions

Add a new page to let the user review and confirm actions, after
dependencies have been added and problems solved.

Ideally, this would re-use the picker-page grid to present these actions,
but for the moment just write it as text (as we did before in the prereq
page) (This is still a slight improvement as it shows all actions for
review, not just the ones added by the solver)
---
 Makefile.am |   2 +
 confirm.cc  | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 confirm.h   |  34 +++++++++++++++
 main.cc     |   4 ++
 prereq.cc   |  41 ++----------------
 prereq.h    |   2 -
 res.rc      |  16 +++++++
 resource.h  |   2 +
 8 files changed, 199 insertions(+), 39 deletions(-)
 create mode 100644 confirm.cc
 create mode 100644 confirm.h

diff --git a/Makefile.am b/Makefile.am
index db8d070..37341b9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -119,6 +119,8 @@ inilint_SOURCES = \
 	compress_gz.h \
 	compress_xz.cc \
 	compress_xz.h \
+	confirm.cc \
+	confirm.h \
 	ConnectionSetting.cc \
 	ConnectionSetting.h \
 	ControlAdjuster.cc \
diff --git a/confirm.cc b/confirm.cc
new file mode 100644
index 0000000..7f8f5da
--- /dev/null
+++ b/confirm.cc
@@ -0,0 +1,137 @@
+/*
+ * Copyright (c) 2018
+ *
+ *     This program is free software; you can redistribute it and/or modify
+ *     it under the terms of the GNU General Public License as published by
+ *     the Free Software Foundation; either version 2 of the License, or
+ *     (at your option) any later version.
+ *
+ *     A copy of the GNU General Public License can be found at
+ *     http://www.gnu.org/
+ *
+ */
+
+#include "confirm.h"
+#include "threebar.h"
+#include "resource.h"
+#include "state.h"
+#include "ControlAdjuster.h"
+#include "package_db.h"
+#include "package_meta.h"
+
+extern ThreeBarProgressPage Progress;
+
+// Sizing information.
+static ControlAdjuster::ControlInfo ConfirmControlsInfo[] = {
+  {IDC_CONFIRM_EDIT, CP_STRETCH, CP_STRETCH},
+  {0, CP_LEFT, CP_TOP}
+};
+
+// ---------------------------------------------------------------------------
+// implements class ConfirmPage
+// ---------------------------------------------------------------------------
+
+ConfirmPage::ConfirmPage ()
+{
+  sizeProcessor.AddControlInfo (ConfirmControlsInfo);
+}
+
+bool
+ConfirmPage::Create ()
+{
+  return PropertyPage::Create (IDD_CONFIRM);
+}
+
+void
+ConfirmPage::OnInit ()
+{
+  // set the edit-area to a larger font
+  SetDlgItemFont(IDC_CONFIRM_EDIT, "MS Shell Dlg", 10);
+}
+
+void
+ConfirmPage::OnActivate()
+{
+  // generate a report on actions we're going to take
+  std::string s = "";
+
+  // first list things we will erase
+  packagedb db;
+  const SolverTransactionList & trans = db.solution.transactions ();
+  for (SolverTransactionList::const_iterator i = trans.begin ();
+       i != trans.end (); i++)
+    {
+      if (i->type == SolverTransaction::transErase)
+          {
+            packageversion pv = i->version;
+            packagemeta *pkg = db.findBinary (PackageSpecification (pv.Name ()));
+
+            s += "Uninstall ";
+            s += i->version.Name();
+            s += " ";
+            s += i->version.Canonical_version();
+            if (pkg->desired)
+              s += " (automatically added)";
+            s += "\r\n";
+          }
+    }
+
+  // then list things installed
+  for (SolverTransactionList::const_iterator i = trans.begin ();
+       i != trans.end (); i++)
+    {
+      packageversion pv = i->version;
+      packagemeta *pkg = db.findBinary (PackageSpecification (pv.Name ()));
+
+      if (i->type == SolverTransaction::transInstall)
+          {
+            s += "Install ";
+            s += i->version.Name();
+            s += " ";
+            s += i->version.Canonical_version();
+            if (!pkg->desired)
+              s += " (automatically added)";
+            s += "\r\n";
+          }
+    }
+
+  SetDlgItemText (GetHWND (), IDC_CONFIRM_EDIT, s.c_str ());
+
+  // move focus to 'next' button, so enter doesn't get eaten by edit control
+  HWND nextButton = ::GetDlgItem(::GetParent(GetHWND()), 0x3024 /* ID_WIZNEXT */);
+  PostMessage (GetHWND (), WM_NEXTDLGCTL, (WPARAM)nextButton, TRUE);
+}
+
+long
+ConfirmPage::OnNext ()
+{
+  return whatNext();
+}
+
+long
+ConfirmPage::whatNext ()
+{
+  if (source == IDC_SOURCE_LOCALDIR)
+    {
+      // Next, install
+      Progress.SetActivateTask (WM_APP_START_INSTALL);
+    }
+  else
+    {
+      // Next, start download from internet
+      Progress.SetActivateTask (WM_APP_START_DOWNLOAD);
+    }
+  return IDD_INSTATUS;
+}
+
+long
+ConfirmPage::OnBack ()
+{
+  return IDD_CHOOSE;
+}
+
+long
+ConfirmPage::OnUnattended ()
+{
+  return whatNext();
+}
diff --git a/confirm.h b/confirm.h
new file mode 100644
index 0000000..fe11ee0
--- /dev/null
+++ b/confirm.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2018
+ *
+ *     This program is free software; you can redistribute it and/or modify
+ *     it under the terms of the GNU General Public License as published by
+ *     the Free Software Foundation; either version 2 of the License, or
+ *     (at your option) any later version.
+ *
+ *     A copy of the GNU General Public License can be found at
+ *     http://www.gnu.org/
+ *
+ */
+
+#ifndef SETUP_CONFIRM_H
+#define SETUP_CONFIRM_H
+
+#include "proppage.h"
+
+class ConfirmPage:public PropertyPage
+{
+public:
+  ConfirmPage ();
+  virtual ~ConfirmPage () { };
+  bool Create ();
+  virtual void OnInit ();
+  virtual void OnActivate ();
+  virtual long OnNext ();
+  virtual long OnBack ();
+  virtual long OnUnattended ();
+private:
+  long whatNext ();
+};
+
+#endif /* SETUP_CONFIRM_H */
diff --git a/main.cc b/main.cc
index 028f8de..2e57f94 100644
--- a/main.cc
+++ b/main.cc
@@ -55,6 +55,7 @@
 #include "site.h"
 #include "choose.h"
 #include "prereq.h"
+#include "confirm.h"
 #include "threebar.h"
 #include "desktop.h"
 #include "postinstallresults.h"
@@ -135,6 +136,7 @@ main_display ()
   SitePage Site;
   ChooserPage Chooser;
   PrereqPage Prereq;
+  ConfirmPage Confirm;
   DesktopSetupPage Desktop;
   PropSheet MainWindow;
 
@@ -175,6 +177,7 @@ main_display ()
   Site.Create ();
   Chooser.Create ();
   Prereq.Create ();
+  Confirm.Create ();
   Progress.Create ();
   PostInstallResults.Create ();
   Desktop.Create ();
@@ -189,6 +192,7 @@ main_display ()
   MainWindow.AddPage (&Site);
   MainWindow.AddPage (&Chooser);
   MainWindow.AddPage (&Prereq);
+  MainWindow.AddPage (&Confirm);
   MainWindow.AddPage (&Progress);
   MainWindow.AddPage (&PostInstallResults);
   MainWindow.AddPage (&Desktop);
diff --git a/prereq.cc b/prereq.cc
index effa7cc..8fcd3ba 100644
--- a/prereq.cc
+++ b/prereq.cc
@@ -13,26 +13,14 @@
  *
  */
 
-#include "win32.h"
-#include <commctrl.h>
-#include <stdio.h>
-#include <io.h>
-#include <ctype.h>
-#include <process.h>
-#include <queue>
-
 #include "prereq.h"
-#include "dialog.h"
 #include "resource.h"
 #include "state.h"
-#include "propsheet.h"
 #include "threebar.h"
-#include "Generic.h"
 #include "LogSingleton.h"
 #include "ControlAdjuster.h"
 #include "package_db.h"
-#include "package_meta.h"
-#include "msg.h"
+
 #include "Exception.h"
 #include "getopt++/BoolOption.h"
 
@@ -120,23 +108,7 @@ PrereqPage::OnNext ()
   PrereqChecker p;
   p.finalize();
 
-  return whatNext();
-}
-
-long
-PrereqPage::whatNext ()
-{
-  if (source == IDC_SOURCE_LOCALDIR)
-    {
-      // Next, install
-      Progress.SetActivateTask (WM_APP_START_INSTALL);
-    }
-  else
-    {
-      // Next, start download from internet
-      Progress.SetActivateTask (WM_APP_START_DOWNLOAD);
-    }
-  return IDD_INSTATUS;
+  return IDD_CONFIRM;
 }
 
 long
@@ -160,7 +132,7 @@ PrereqPage::OnUnattended ()
   if (unattended_mode == chooseronly)
     return -1;
 
-  return whatNext();
+  return IDD_CONFIRM;
 }
 
 // ---------------------------------------------------------------------------
@@ -233,12 +205,7 @@ do_prereq_check_thread(HINSTANCE h, HWND owner)
   if (p.isMet ())
     {
       p.finalize();
-
-      if (source == IDC_SOURCE_LOCALDIR)
-	Progress.SetActivateTask (WM_APP_START_INSTALL);  // install
-      else
-	Progress.SetActivateTask (WM_APP_START_DOWNLOAD); // start download
-      retval = IDD_INSTATUS;
+      retval = IDD_CONFIRM;
     }
   else
     {
diff --git a/prereq.h b/prereq.h
index 24e6de5..749d3eb 100644
--- a/prereq.h
+++ b/prereq.h
@@ -27,8 +27,6 @@ public:
   virtual long OnNext ();
   virtual long OnBack ();
   virtual long OnUnattended ();
-private:
-  long whatNext ();
 };
 
 class PrereqChecker
diff --git a/res.rc b/res.rc
index 3881f14..745b396 100644
--- a/res.rc
+++ b/res.rc
@@ -392,6 +392,22 @@ BEGIN
 
 END
 
+IDD_CONFIRM DIALOG DISCARDABLE  0, 0, SETUP_STANDARD_DIALOG_DIMS
+STYLE DS_MODALFRAME | DS_3DLOOK | WS_CHILD | WS_VISIBLE | WS_CAPTION |
+    WS_SYSMENU
+CAPTION "Cygwin Setup - Review and confirm changes"
+FONT 8, "MS Shell Dlg"
+BEGIN
+    CONTROL         "",IDC_HEADSEPARATOR,"Static",SS_BLACKFRAME | SS_SUNKEN,
+                    0,28,SETUP_STANDARD_DIALOG_W,1
+    ICON            IDI_CYGWIN,IDC_HEADICON,SETUP_HEADICON_X,0,21,20
+    LTEXT           "Review and confirm changes",IDC_STATIC_HEADER_TITLE
+                    ,7,0,258,8,NOT WS_GROUP
+    EDITTEXT        IDC_CONFIRM_EDIT,7,41,325,131,WS_VSCROLL | WS_HSCROLL |
+                    ES_LEFT | ES_MULTILINE | ES_READONLY | ES_AUTOHSCROLL |
+                    ES_AUTOVSCROLL
+END
+
 IDD_DROPPED DIALOG DISCARDABLE  0, 0, SETUP_STANDARD_DIALOG_W, 142
 STYLE DS_MODALFRAME | DS_CENTER | WS_POPUP | WS_CAPTION | WS_SYSMENU
 CAPTION "Cygwin Setup - Use dropped mirrors?"
diff --git a/resource.h b/resource.h
index 5294e38..31e080f 100644
--- a/resource.h
+++ b/resource.h
@@ -69,6 +69,7 @@
 #define IDD_POSTINSTALL                   222
 #define IDD_FILE_INUSE                    223
 #define IDD_DOWNLOAD_ERROR                224
+#define IDD_CONFIRM                       225
 
 // Bitmaps
 
@@ -181,3 +182,4 @@
 #define IDC_DOWNLOAD_EDIT                 594
 #define IDC_CHOOSE_DO_SEARCH              595
 #define IDC_CHOOSE_SYNC                   596
+#define IDC_CONFIRM_EDIT                  597
-- 
2.15.1

Re: [PATCH setup libsolv, v2] Let the user review added dependencies

[Ken Brown]

On 1/24/2018 3:30 PM, Jon Turney wrote:
> On 22/01/2018 17:37, Ken Brown wrote:
>> If the solver found no problems but added packages to resolve
>> dependencies, ask the user whether they want to review the added
>> packages before proceeding.
>>
>> If they answer Yes, go back to the chooser with the 'Pending' view
>> selected.  The implementation adds several new members to the
>> PrereqChecker class so that the latter can communicate with the
>> chooser page.
> 
> As discussed, this approach could be confusing.
> 
> Attached is a slightly different approach, which adds a new page to 
> review and confirm what actions we're going to take.
> 
> For the moment, this just contains a simple text report, but I guess 
> this could be extended e.g. to use a grid control, or give reasons for 
> why packages are being installed.

I like this a lot.  It's *much* better than my approach.

My only suggestion is that you rethink what the confirm page should do 
in "download only" mode.  Maybe that page could even be skipped.  But if 
you think the user should confirm the download, there's no need to 
mention "uninstall" actions, and "install" actions should be reported as 
"download".

Ken

Re: [PATCH setup libsolv, v2] Let the user review added dependencies

[Ken Brown]

On 1/24/2018 10:25 PM, Ken Brown wrote:
> On 1/24/2018 3:30 PM, Jon Turney wrote:
>> On 22/01/2018 17:37, Ken Brown wrote:
>>> If the solver found no problems but added packages to resolve
>>> dependencies, ask the user whether they want to review the added
>>> packages before proceeding.
>>>
>>> If they answer Yes, go back to the chooser with the 'Pending' view
>>> selected.  The implementation adds several new members to the
>>> PrereqChecker class so that the latter can communicate with the
>>> chooser page.
>>
>> As discussed, this approach could be confusing.
>>
>> Attached is a slightly different approach, which adds a new page to 
>> review and confirm what actions we're going to take.
>>
>> For the moment, this just contains a simple text report, but I guess 
>> this could be extended e.g. to use a grid control, or give reasons for 
>> why packages are being installed.
> 
> I like this a lot.  It's *much* better than my approach.
> 
> My only suggestion is that you rethink what the confirm page should do 
> in "download only" mode.  Maybe that page could even be skipped.  But if 
> you think the user should confirm the download, there's no need to 
> mention "uninstall" actions, and "install" actions should be reported as 
> "download".

One other minor detail: What should happen if nothing is being done?  I 
don't think we necessarily want to skip the confirm page, because maybe 
the user was expecting something to happen.  But it looks a little 
strange to just display an empty report.  Maybe the report should say 
something like "No changes will be made" or, in download mode, "Nothing 
will be downloaded".

Ken

Re: [PATCH setup libsolv, v2] Let the user review added dependencies

[Jon Turney]

On 25/01/2018 03:57, Ken Brown wrote:
> On 1/24/2018 10:25 PM, Ken Brown wrote:
>> On 1/24/2018 3:30 PM, Jon Turney wrote:
>>> On 22/01/2018 17:37, Ken Brown wrote:
>>>> If the solver found no problems but added packages to resolve
>>>> dependencies, ask the user whether they want to review the added
>>>> packages before proceeding.
>>>>
>>>> If they answer Yes, go back to the chooser with the 'Pending' view
>>>> selected.  The implementation adds several new members to the
>>>> PrereqChecker class so that the latter can communicate with the
>>>> chooser page.
>>>
>>> As discussed, this approach could be confusing.
>>>
>>> Attached is a slightly different approach, which adds a new page to 
>>> review and confirm what actions we're going to take.
>>>
>>> For the moment, this just contains a simple text report, but I guess 
>>> this could be extended e.g. to use a grid control, or give reasons 
>>> for why packages are being installed.
>>
>> I like this a lot.  It's *much* better than my approach.
>>
>> My only suggestion is that you rethink what the confirm page should do 
>> in "download only" mode.  Maybe that page could even be skipped.  But 
>> if you think the user should confirm the download, there's no need to 
>> mention "uninstall" actions, and "install" actions should be reported 
>> as "download".
> 
> One other minor detail: What should happen if nothing is being done?  I 
> don't think we necessarily want to skip the confirm page, because maybe 
> the user was expecting something to happen.  But it looks a little 
> strange to just display an empty report.  Maybe the report should say 
> something like "No changes will be made" or, in download mode, "Nothing 
> will be downloaded".

Good points. I adjusted the output as you suggest.