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

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Ken Brown-6
---
 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

Reply | Threaded
Open this post in threaded view
|

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.

Reply | Threaded
Open this post in threaded view
|

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

Ken Brown-6
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
Reply | Threaded
Open this post in threaded view
|

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