* [PATCH setup libsolv, v2] Let the user review added dependencies @ 2018-01-22 17:37 Ken Brown 2018-01-22 21:51 ` Brian Inglis 2018-01-24 20:30 ` Jon Turney 0 siblings, 2 replies; 9+ messages in thread From: Ken Brown @ 2018-01-22 17:37 UTC (permalink / raw) To: cygwin-apps 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH setup libsolv, v2] Let the user review added dependencies 2018-01-22 17:37 [PATCH setup libsolv, v2] Let the user review added dependencies Ken Brown @ 2018-01-22 21:51 ` Brian Inglis 2018-01-22 22:40 ` Ken Brown 2018-01-24 20:30 ` Jon Turney 1 sibling, 1 reply; 9+ messages in thread From: Brian Inglis @ 2018-01-22 21:51 UTC (permalink / raw) To: cygwin-apps 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH setup libsolv, v2] Let the user review added dependencies 2018-01-22 21:51 ` Brian Inglis @ 2018-01-22 22:40 ` Ken Brown 2018-01-23 1:07 ` Brian Inglis 0 siblings, 1 reply; 9+ messages in thread From: Ken Brown @ 2018-01-22 22:40 UTC (permalink / raw) To: cygwin-apps 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH setup libsolv, v2] Let the user review added dependencies 2018-01-22 22:40 ` Ken Brown @ 2018-01-23 1:07 ` Brian Inglis 2018-01-23 15:05 ` cyg Simple 0 siblings, 1 reply; 9+ messages in thread From: Brian Inglis @ 2018-01-23 1:07 UTC (permalink / raw) To: cygwin-apps 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH setup libsolv, v2] Let the user review added dependencies 2018-01-23 1:07 ` Brian Inglis @ 2018-01-23 15:05 ` cyg Simple 0 siblings, 0 replies; 9+ messages in thread From: cyg Simple @ 2018-01-23 15:05 UTC (permalink / raw) To: cygwin-apps 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH setup libsolv, v2] Let the user review added dependencies 2018-01-22 17:37 [PATCH setup libsolv, v2] Let the user review added dependencies Ken Brown 2018-01-22 21:51 ` Brian Inglis @ 2018-01-24 20:30 ` Jon Turney 2018-01-25 3:25 ` Ken Brown 1 sibling, 1 reply; 9+ messages in thread From: Jon Turney @ 2018-01-24 20:30 UTC (permalink / raw) To: cygwin-apps [-- 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH setup libsolv, v2] Let the user review added dependencies 2018-01-24 20:30 ` Jon Turney @ 2018-01-25 3:25 ` Ken Brown 2018-01-25 3:57 ` Ken Brown 0 siblings, 1 reply; 9+ messages in thread From: Ken Brown @ 2018-01-25 3:25 UTC (permalink / raw) To: cygwin-apps 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH setup libsolv, v2] Let the user review added dependencies 2018-01-25 3:25 ` Ken Brown @ 2018-01-25 3:57 ` Ken Brown 2018-01-28 18:11 ` Jon Turney 0 siblings, 1 reply; 9+ messages in thread From: Ken Brown @ 2018-01-25 3:57 UTC (permalink / raw) To: cygwin-apps 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH setup libsolv, v2] Let the user review added dependencies 2018-01-25 3:57 ` Ken Brown @ 2018-01-28 18:11 ` Jon Turney 0 siblings, 0 replies; 9+ messages in thread From: Jon Turney @ 2018-01-28 18:11 UTC (permalink / raw) To: cygwin-apps 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-01-28 18:11 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-01-22 17:37 [PATCH setup libsolv, v2] Let the user review added dependencies Ken Brown 2018-01-22 21:51 ` Brian Inglis 2018-01-22 22:40 ` Ken Brown 2018-01-23 1:07 ` Brian Inglis 2018-01-23 15:05 ` cyg Simple 2018-01-24 20:30 ` Jon Turney 2018-01-25 3:25 ` Ken Brown 2018-01-25 3:57 ` Ken Brown 2018-01-28 18:11 ` Jon Turney
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).