Improve Qt version page

* Fix changing version page in case of error.
* Suppress error message if we try to remove a Qt version
  which does not (yet) exist in our registry hive.
* Do Qt version validation a bit earlier so we can report errors
  in a timely manner and display a message box to the user.

Fixes: QTVSADDINBUG-1058
Fixes: QTVSADDINBUG-1064
Change-Id: I0523ca8be9245157b9a0e65a5da056f7c79b1e04
Reviewed-by: Miguel Costa <miguel.costa@qt.io>

2 files changed, 58 insertions, 35 deletions

diff --git a/QtVsTools.Core/QtVersionManager.cs b/QtVsTools.Core/QtVersionManager.cs
index 1c85603a..eb398216 100644
--- a/QtVsTools.Core/QtVersionManager.cs
+++ b/QtVsTools.Core/QtVersionManager.cs
@@ -263,6 +263,14 @@ namespace QtVsTools.Core
             return true;
         }
 
+        public bool HasVersion(string versionName)
+        {
+            if (string.IsNullOrEmpty(versionName))
+                return false;
+            return Registry.CurrentUser.OpenSubKey(Path.Combine("SOFTWARE", regVersionPath,
+                versionName), false) != null;
+        }
+
         public void RemoveVersion(string versionName)
         {
             var key = Registry.CurrentUser.OpenSubKey("SOFTWARE" + Path.DirectorySeparatorChar
diff --git a/QtVsTools.Package/Options/QtVersionsPage.cs b/QtVsTools.Package/Options/QtVersionsPage.cs
index 5df2e25f..7eb98fd6 100644
--- a/QtVsTools.Package/Options/QtVersionsPage.cs
+++ b/QtVsTools.Package/Options/QtVersionsPage.cs
@@ -92,13 +92,14 @@ namespace QtVsTools.Options
             void RemoveVersion(string versionName)
             {
                 try {
-                    VersionManager.RemoveVersion(versionName);
+                    if (VersionManager.HasVersion(versionName))
+                        VersionManager.RemoveVersion(versionName);
                 } catch (Exception exception) {
                     exception.Log();
                 }
             }
 
-            var versions = VersionsTable.Versions;
+            var versions = VersionsTable.Versions.ToList();
             foreach (var version in versions) {
                 if (version.State.HasFlag(State.Removed))
                     RemoveVersion(version.VersionName);
@@ -107,42 +108,27 @@ namespace QtVsTools.Options
                     continue;
 
                 try {
-                    if (version.Host == BuildHost.Windows) {
-                        if (version.State.HasFlag((State)Column.Path)) {
-                            var versionPath = version.Path;
-                            var ignoreCase = StringComparison.CurrentCultureIgnoreCase;
-                            if (Path.GetFileName(versionPath).Equals("qmake.exe", ignoreCase))
-                                versionPath = Path.GetDirectoryName(versionPath);
-                            if (Path.GetFileName(versionPath).Equals("bin", ignoreCase))
-                                versionPath = Path.GetDirectoryName(versionPath);
-                            var versionInfo = VersionInformation.Get(versionPath);
-                            var generator = versionInfo.GetQMakeConfEntry("MAKEFILE_GENERATOR");
-                            if (generator != "MSVC.NET" && generator != "MSBUILD")
-                                throw new Exception(string.Format(
-                                    "This Qt version uses an unsupported makefile generator (used: "
-                                    + "{0}, supported: MSVC.NET, MSBUILD)", generator));
-                            VersionManager.SaveVersion(version.VersionName, versionPath);
-                        }
-                    } else {
+                    if (version.Host != BuildHost.Windows) {
                         string name = version.VersionName;
-                        string access =
-                            (version.Host == BuildHost.LinuxSSH) ? "SSH" : "WSL";
+                        string access = version.Host == BuildHost.LinuxSSH ? "SSH" : "WSL";
                         string path = version.Path;
                         string compiler = version.Compiler;
                         if (compiler == "g++")
                             compiler = string.Empty;
-                        path = string.Format("{0}:{1}:{2}", access, path, compiler);
-                        VersionManager.SaveVersion(name, path, checkPath: false);
+                        VersionManager.SaveVersion(name, $"{access}:{path}:{compiler}",
+                            checkPath: false);
+                    } else {
+                        if (version.State.HasFlag((State)Column.Path))
+                            VersionManager.SaveVersion(version.VersionName, version.Path);
                     }
 
                     if (version.State.HasFlag((State)Column.VersionName)) {
                         try {
                             VersionManager.SaveVersion(version.VersionName, version.Path);
-                            if (!string.IsNullOrEmpty(version.InitialVersionName))
-                                VersionManager.RemoveVersion(version.InitialVersionName);
                         } catch (Exception exception) {
                             exception.Log();
                         }
+                        RemoveVersion(version.InitialVersionName);
                     }
                 } catch (Exception exception) {
                     exception.Log();
@@ -166,18 +152,47 @@ namespace QtVsTools.Options
 
         protected override void OnApply(PageApplyEventArgs e)
         {
-            var errorMessages = VersionsTable.GetErrorMessages();
-            if (errorMessages == null || !errorMessages.Any()) {
+            var errorMessages = VersionsTable.GetErrorMessages().ToList();
+
+            try {
+                var versions = VersionsTable.Versions;
+                foreach (var version in versions) {
+                    if (!version.State.HasFlag(State.Modified) || version.Host != BuildHost.Windows)
+                        continue;
+                    if (version.State.HasFlag((State)Column.Path)) {
+                        var versionPath = version.Path;
+                        var ignoreCase = StringComparison.OrdinalIgnoreCase;
+                        if (Path.GetFileName(versionPath).Equals("qmake.exe", ignoreCase))
+                            versionPath = Path.GetDirectoryName(versionPath);
+                        if (Path.GetFileName(versionPath ?? "").Equals("bin", ignoreCase))
+                            versionPath = Path.GetDirectoryName(versionPath);
+
+                        QMakeConf qtConfiguration = new QMakeConf(versionPath);
+                        var generator = qtConfiguration.Entries["MAKEFILE_GENERATOR"].ToString();
+
+                        if (generator != "MSVC.NET" && generator != "MSBUILD")
+                            errorMessages.Add($"Unsupported makefile generator used: {generator}");
+                    }
+                }
+            } catch (Exception exception) {
+                errorMessages.Add(exception.Message);
+            }
+
+            if (errorMessages.Any()) {
+                VersionsTable.Focus();
+                var errorMessage = "Invalid Qt versions:\r\n"
+                    + $"{string.Join("\r\n", errorMessages.Select(errMsg => " * " + errMsg))}";
+                VsShellUtilities.ShowMessageBox(
+                    QtVsToolsPackage.Instance,
+                    errorMessage,
+                    "Qt VS Tools",
+                    OLEMSGICON.OLEMSGICON_WARNING,
+                    OLEMSGBUTTON.OLEMSGBUTTON_OK,
+                    OLEMSGDEFBUTTON.OLEMSGDEFBUTTON_FIRST);
+                e.ApplyBehavior = ApplyKind.CancelNoNavigate;
+            } else {
                 base.OnApply(e);
-                return;
             }
-            e.ApplyBehavior = ApplyKind.Cancel;
-            VersionsTable.Focus();
-            string errorMessage = string.Format("Invalid Qt versions:\r\n{0}",
-                    string.Join("\r\n", errorMessages.Select(errMsg => " * " + errMsg)));
-            VsShellUtilities.ShowMessageBox(QtVsToolsPackage.Instance,
-                errorMessage, "Qt VS Tools", OLEMSGICON.OLEMSGICON_WARNING,
-                OLEMSGBUTTON.OLEMSGBUTTON_OK, OLEMSGDEFBUTTON.OLEMSGDEFBUTTON_FIRST);
         }
     }
 }