[PATCH setup] Sort the packages listed in the "confirm" dialog

[PATCH setup] Sort the packages listed in the "confirm" dialog

[Ken Brown]

---
 confirm.cc | 44 ++++++++++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/confirm.cc b/confirm.cc
index eb6bd99..92360cc 100644
--- a/confirm.cc
+++ b/confirm.cc
@@ -19,6 +19,8 @@
 #include "package_db.h"
 #include "package_meta.h"
 
+#include <algorithm>
+
 extern ThreeBarProgressPage Progress;
 
 // Sizing information.
@@ -61,26 +63,34 @@ ConfirmPage::OnActivate()
   // first list things we will erase
   if (source != IDC_SOURCE_DOWNLOAD)
     {
+      std::vector<std::string> erase;
       for (SolverTransactionList::const_iterator i = trans.begin ();
            i != trans.end (); i++)
         {
           if (i->type == SolverTransaction::transErase)
             {
+              std::string line;
               packageversion pv = i->version;
               packagemeta *pkg = db.findBinary (PackageSpecification (pv.Name ()));
 
-              s += "Uninstall ";
-              s += i->version.Name();
-              s += " ";
-              s += i->version.Canonical_version();
+              line += "Uninstall ";
+              line += i->version.Name();
+              line += " ";
+              line += i->version.Canonical_version();
               if (pkg && pkg->desired)
-                s += " (automatically added)";
-              s += "\r\n";
+                line += " (automatically added)";
+              line += "\r\n";
+              erase.push_back (line);
             }
         }
+      sort (erase.begin(), erase.end());
+      for (std::vector<std::string>::const_iterator i = erase.begin ();
+	   i != erase.end(); i++)
+        s += *i;
     }
 
   // then list things downloaded or installed
+  std::vector<std::string> install;
   for (SolverTransactionList::const_iterator i = trans.begin ();
        i != trans.end (); i++)
     {
@@ -89,20 +99,26 @@ ConfirmPage::OnActivate()
 
       if (i->type == SolverTransaction::transInstall)
           {
+            std::string line;
             if (source != IDC_SOURCE_DOWNLOAD)
-              s += "Install ";
+              line += "Install ";
             else
-              s += "Download ";
-            s += i->version.Name();
-            s += " ";
-            s += i->version.Canonical_version();
+              line += "Download ";
+            line += i->version.Name();
+            line += " ";
+            line += i->version.Canonical_version();
             if (i->version.Type() == package_source)
-              s += " (source)";
+              line += " (source)";
             else if (pkg && !pkg->desired)
-              s += " (automatically added)";
-            s += "\r\n";
+              line += " (automatically added)";
+            line += "\r\n";
+            install.push_back (line);
           }
     }
+  sort (install.begin(), install.end());
+  for (std::vector<std::string>::const_iterator i = install.begin ();
+       i != install.end(); i++)
+    s += *i;
 
   // be explicit about doing nothing
   if (s.empty())
-- 
2.16.2

Re: [PATCH setup] Sort the packages listed in the "confirm" dialog

[Jon Turney]

On 18/03/2018 16:31, Ken Brown wrote:
> ---
>   confirm.cc | 44 ++++++++++++++++++++++++++++++--------------
>   1 file changed, 30 insertions(+), 14 deletions(-)
Applied, thanks.

I know you had a few other patchsets in flight, which I've unfortunately 
taken my eye off.  If it's easier, you could push a branch with 
somewhere with what's outstanding, and I'll see if I can pick them up.

Re: [PATCH setup] Sort the packages listed in the "confirm" dialog

[Ken Brown]

On 6/6/2018 4:44 PM, Jon Turney wrote:
> I know you had a few other patchsets in flight, which I've unfortunately 
> taken my eye off.  If it's easier, you could push a branch with 
> somewhere with what's outstanding, and I'll see if I can pick them up.

Here are the patchsets that I've sent in the last few months:

1. https://sourceware.org/ml/cygwin-apps/2018-03/msg00033.html
This still applies to the current HEAD.

2. https://sourceware.org/ml/cygwin-apps/2018-03/msg00051.html
This also applies to the current HEAD.

3. https://sourceware.org/ml/cygwin-apps/2018-04/msg00002.html
This needs to be tweaked slightly.  I'll send a v2 today or tomorrow.

Next, there's the tentative proposal I made in 
https://sourceware.org/ml/cygwin-apps/2018-04/msg00000.html, with a 
cygport patch that isn't quite right.  That could use some followup 
eventually.

Finally, there's the older patchset attempting to distinguish between 
user URLs and Cygwin mirrors:

   https://sourceware.org/ml/cygwin-apps/2017-12/msg00051.html

with a followup patchset here:

   https://sourceware.org/ml/cygwin-apps/2017-12/msg00066.html

I'm honestly not sure whether this "user URL" idea is worth pursuing, 
because it might end up confusing users more than it helps them.  But if 
you think it is, I'll push it to a branch for further work.

Ken

Re: [PATCH setup] Sort the packages listed in the "confirm" dialog

[Jon Turney]

On 07/06/2018 18:14, Ken Brown wrote:
> On 6/6/2018 4:44 PM, Jon Turney wrote:
>> I know you had a few other patchsets in flight, which I've 
>> unfortunately taken my eye off.  If it's easier, you could push a 
>> branch with somewhere with what's outstanding, and I'll see if I can 
>> pick them up.
> 
> Here are the patchsets that I've sent in the last few months:

Thanks very much for the summary.  I think I'm more or less caught up now.

I'd like to do another setup release soonish to get your useful fixes 
deployed, so please let me know if there's anything I've missed.

> 1. https://sourceware.org/ml/cygwin-apps/2018-03/msg00033.html > This still applies to the current HEAD.

Applied 1/3, thanks.  I've posted a suggestion for reworking 3/3.

"Improve the handling of packagesource::sites"

> 2. https://sourceware.org/ml/cygwin-apps/2018-03/msg00051.html
> This also applies to the current HEAD.

"setup: uninstalling an orphaned package"

Applied, thanks.

> 3. https://sourceware.org/ml/cygwin-apps/2018-04/msg00002.html
> This needs to be tweaked slightly.  I'll send a v2 today or tomorrow.

"Improve the handling of command line package selection"

Applied, thanks.

> Next, there's the tentative proposal I made in 
> https://sourceware.org/ml/cygwin-apps/2018-04/msg00000.html, with a 
> cygport patch that isn't quite right.  That could use some followup 
> eventually.

Thanks.  I think this shouldn't need any more setup changes to support.

> Finally, there's the older patchset attempting to distinguish between 
> user URLs and Cygwin mirrors:
> 
>    https://sourceware.org/ml/cygwin-apps/2017-12/msg00051.html
> 
> with a followup patchset here:
> 
>    https://sourceware.org/ml/cygwin-apps/2017-12/msg00066.html
> 
> I'm honestly not sure whether this "user URL" idea is worth pursuing, 
> because it might end up confusing users more than it helps them.  But if 
> you think it is, I'll push it to a branch for further work.

Yeah, I tend to agree.  We could be doing something better, but it's not 
very clear yet what that is :S