Fixes after CR

Most important: 
Change Download dialog to select path there.
Every user query triggers new download of version file, possibly stops former download (with dialog).
Some functions refactored.

src/slic3r/GUI/GUI.cpp

@@ -476,25 +476,36 @@ void about()
 
 void desktop_open_datadir_folder()
 {
-	// Execute command to open a file explorer, platform dependent.
+	boost::filesystem::path path(data_dir());
-	// FIXME: The const_casts aren't needed in wxWidgets 3.1, remove them when we upgrade.
+	desktop_open_folder(std::move(path));
+}
 
-	const auto path = data_dir();
+void desktop_open_folder(const boost::filesystem::path& path)
+{
+	if (!boost::filesystem::is_directory(path)) 
+		return;
+
+	// Execute command to open a file explorer, platform dependent.
 #ifdef _WIN32
-		const wxString widepath = from_u8(path);
+	const wxString widepath = path.wstring();
 	const wchar_t* argv[] = { L"explorer", widepath.GetData(), nullptr };
 	::wxExecute(const_cast<wchar_t**>(argv), wxEXEC_ASYNC, nullptr);
 #elif __APPLE__
-		const char *argv[] = { "open", path.data(), nullptr };
+	const char* argv[] = { "open", path.string(), nullptr };
 	::wxExecute(const_cast<char**>(argv), wxEXEC_ASYNC, nullptr);
 #else
-		const char *argv[] = { "xdg-open", path.data(), nullptr };
+	const char* argv[] = { "xdg-open", path.string().c_str(), nullptr };
+	desktop_execute(argv);
+#endif
+}
 
-		// Check if we're running in an AppImage container, if so, we need to remove AppImage's env vars,
+#ifdef __linux__
+namespace {
+wxExecuteEnv get_appimage_exec_env()
+{
+	// If we're running in an AppImage container, we need to remove AppImage's env vars,
 	// because they may mess up the environment expected by the file manager.
 	// Mostly this is about LD_LIBRARY_PATH, but we remove a few more too for good measure.
-		if (wxGetEnv("APPIMAGE", nullptr)) {
-			// We're running from AppImage
 	wxEnvVariableHashMap env_vars;
 	wxGetEnvMap(&env_vars);
 
@@ -513,13 +524,87 @@ void desktop_open_datadir_folder()
 		// set it as CWD for the child process:
 		exec_env.cwd = std::move(owd);
 	}
+	return exec_env;
+}
+} // namespace
+void desktop_execute(const char* argv[])
+{
+	if (sizeof(argv) == 0)
+		return;
 
+	// Check if we're running in an AppImage container, if so, we need to remove AppImage's env vars,
+	// because they may mess up the environment expected by the file manager.
+	// Mostly this is about LD_LIBRARY_PATH, but we remove a few more too for good measure.
+	if (wxGetEnv("APPIMAGE", nullptr)) {
+		// We're running from AppImage
+		wxExecuteEnv exec_env = get_appimage_exec_env();
 		::wxExecute(const_cast<char**>(argv), wxEXEC_ASYNC, nullptr, &exec_env);
-		} else {
+	}
+	else {
 		// Looks like we're NOT running from AppImage, we'll make no changes to the environment.
 		::wxExecute(const_cast<char**>(argv), wxEXEC_ASYNC, nullptr, nullptr);
 	}
-#endif
 }
+void desktop_execute_get_result(wxString command, wxArrayString& output)
+{
+	output.Clear();
+   //Check if we're running in an AppImage container, if so, we need to remove AppImage's env vars,
+   // because they may mess up the environment expected by the file manager.
+   // Mostly this is about LD_LIBRARY_PATH, but we remove a few more too for good measure.
+	if (wxGetEnv("APPIMAGE", nullptr)) {
+		// We're running from AppImage
+		wxExecuteEnv exec_env = get_appimage_exec_env();
+		::wxExecute(command, output, 0, &exec_env);
+	} else {
+		// Looks like we're NOT running from AppImage, we'll make no changes to the environment.
+		::wxExecute(command, output);
+	}
+}
+#endif // __linux__
 
-} }
+#ifdef _WIN32
+bool create_process(const boost::filesystem::path& path, const std::wstring& cmd_opt, std::string& error_msg)
+{
+	// find updater exe
+	if (boost::filesystem::exists(path)) {
+		// Using quoted string as mentioned in CreateProcessW docs.
+		std::wstring wcmd = L"\"" + path.wstring() + L"\"";
+		if (!cmd_opt.empty())
+			wcmd += L" " + cmd_opt;
+
+		// additional information
+		STARTUPINFOW si;
+		PROCESS_INFORMATION pi;
+
+		// set the size of the structures
+		ZeroMemory(&si, sizeof(si));
+		si.cb = sizeof(si);
+		ZeroMemory(&pi, sizeof(pi));
+
+		// start the program up
+		if (CreateProcessW(NULL,   // the path
+			wcmd.data(),    // Command line
+			NULL,           // Process handle not inheritable
+			NULL,           // Thread handle not inheritable
+			FALSE,          // Set handle inheritance to FALSE
+			0,              // No creation flags
+			NULL,           // Use parent's environment block
+			NULL,           // Use parent's starting directory 
+			&si,            // Pointer to STARTUPINFO structure
+			&pi             // Pointer to PROCESS_INFORMATION structure (removed extra parentheses)
+		)) {
+			// Close process and thread handles.
+			CloseHandle(pi.hProcess);
+			CloseHandle(pi.hThread);
+			return true;
+		}
+		else
+			error_msg = "CreateProcessW failed to create process " + boost::nowide::narrow(path.wstring());
+	}
+	else
+		error_msg = "Executable doesn't exists. Path: " + boost::nowide::narrow(path.wstring());
+	return false;
+}
+#endif //_WIN32
+
+} } // namespaces GUI / Slic3r

src/slic3r/GUI/GUI.hpp

@@ -80,6 +80,24 @@ boost::filesystem::path	into_path(const wxString &str);
 extern void about();
 // Ask the destop to open the datadir using the default file explorer.
 extern void desktop_open_datadir_folder();
+// Ask the destop to open the directory specified by path using the default file explorer.
+void desktop_open_folder(const boost::filesystem::path& path);
+
+#ifdef __linux__
+// Calling wxExecute on Linux with proper handling of AppImage's env vars.
+// argv example: { "xdg-open", path.c_str(), nullptr }
+void desktop_execute(const char* argv[]);
+void desktop_execute_get_result(wxString command, wxArrayString& output);
+#endif // __linux__
+
+#ifdef _WIN32
+// Call CreateProcessW to start external proccess on path
+// returns true on success
+// path should contain path to the process
+// cmd_opt can be empty or contain command line options. Example: L"/silent"
+// error_msg will contain error message if create_process return false
+bool create_process(const boost::filesystem::path& path, const std::wstring& cmd_opt, std::string& error_msg);
+#endif //_WIN32
 
 } // namespace GUI
 } // namespace Slic3r

src/slic3r/GUI/GUI_App.cpp

@@ -431,49 +431,20 @@ bool static check_old_linux_datadir(const wxString& app_name) {
 }
 #endif
 
-
 #ifdef _WIN32
+#if 0 // External Updater is replaced with AppUpdater.cpp
 static bool run_updater_win()
 {
     // find updater exe
     boost::filesystem::path path_updater = boost::dll::program_location().parent_path() / "prusaslicer-updater.exe";
-    if (boost::filesystem::exists(path_updater)) {
     // run updater. Original args: /silent -restartapp prusa-slicer.exe -startappfirst
-
+    std::string msg;
-        // Using quoted string as mentioned in CreateProcessW docs, silent execution parameter.
+    bool res = create_process(path_updater, L"/silent", msg);
-        std::wstring wcmd = L"\"" + path_updater.wstring() + L"\" /silent";
+    if (!res)
-
+        BOOST_LOG_TRIVIAL(error) << msg; 
-        // additional information
+    return res;
-        STARTUPINFOW si;
-        PROCESS_INFORMATION pi;
-
-        // set the size of the structures
-        ZeroMemory(&si, sizeof(si));
-        si.cb = sizeof(si);
-        ZeroMemory(&pi, sizeof(pi));
-
-        // start the program up
-        if (CreateProcessW(NULL,   // the path
-            wcmd.data(),    // Command line
-            NULL,           // Process handle not inheritable
-            NULL,           // Thread handle not inheritable
-            FALSE,          // Set handle inheritance to FALSE
-            0,              // No creation flags
-            NULL,           // Use parent's environment block
-            NULL,           // Use parent's starting directory 
-            &si,            // Pointer to STARTUPINFO structure
-            &pi             // Pointer to PROCESS_INFORMATION structure (removed extra parentheses)
-        )) {
-            // Close process and thread handles.
-            CloseHandle(pi.hProcess);
-            CloseHandle(pi.hThread);
-            return true;
-        } else {
-            BOOST_LOG_TRIVIAL(error) << "Failed to start prusaslicer-updater.exe with command " << wcmd;
-        }
-    }
-    return false;
 }
+#endif // 0
 #endif // _WIN32
 
 struct FileWildcards {
@@ -817,7 +788,7 @@ void GUI_App::post_init()
         CallAfter([this] {
             bool cw_showed = this->config_wizard_startup();
             this->preset_updater->sync(preset_bundle);
-            this->app_version_check();
+            this->app_version_check(false);
             if (! cw_showed) {
                 // The CallAfter is needed as well, without it, GL extensions did not show.
                 // Also, we only want to show this when the wizard does not, so the new user
@@ -1242,7 +1213,6 @@ bool GUI_App::on_init_inner()
         preset_updater = new PresetUpdater();
         Bind(EVT_SLIC3R_VERSION_ONLINE, &GUI_App::on_version_read, this);
         Bind(EVT_SLIC3R_EXPERIMENTAL_VERSION_ONLINE, [this](const wxCommandEvent& evt) {
-            app_config->save();//lm:What is the purpose?
             if (this->plater_ != nullptr && app_config->get("notify_release") == "all") {
                 std::string evt_string = into_u8(evt.GetString());
                 if (*Semver::parse(SLIC3R_VERSION) < *Semver::parse(evt_string)) {
@@ -1265,10 +1235,13 @@ bool GUI_App::on_init_inner()
         Bind(EVT_SLIC3R_APP_DOWNLOAD_FAILED, [this](const wxCommandEvent& evt) {
             if (this->plater_ != nullptr)
                 this->plater_->get_notification_manager()->close_notification_of_type(NotificationType::AppDownload);
+            if(!evt.GetString().IsEmpty())
                 show_error(nullptr, evt.GetString());
         });
 
-        
+        Bind(EVT_SLIC3R_APP_OPEN_FAILED, [this](const wxCommandEvent& evt) {
+            show_error(nullptr, evt.GetString());
+        });
     }
     else {
 #ifdef __WXMSW__ 
@@ -2326,7 +2299,7 @@ void GUI_App::add_config_menu(wxMenuBar *menu)
 			check_updates(true);
 			break;
         case ConfigMenuUpdateApp:
-            app_updater(true);
+            app_version_check(true);
             break;
 #ifdef __linux__
         case ConfigMenuDesktopIntegration:
@@ -3270,7 +3243,6 @@ void GUI_App::associate_gcode_files()
 
 void GUI_App::on_version_read(wxCommandEvent& evt)
 {
-    
     app_config->set("version_online", into_u8(evt.GetString()));
     app_config->save();
     std::string opt = app_config->get("notify_release");
@@ -3290,8 +3262,8 @@ void GUI_App::on_version_read(wxCommandEvent& evt)
     );
     */
     // updater 
-
+    // read triggered_by_user that was set when calling  GUI_App::app_version_check
-    app_updater(false);
+    app_updater(m_app_updater->get_triggered_by_user());
 }
 
 void GUI_App::app_updater(bool from_user)
@@ -3321,43 +3293,15 @@ void GUI_App::app_updater(bool from_user)
     if (dialog_result != wxID_OK) {
         return;
     }
-    // dialog with new version download (installer or app dependent on system)
+    // dialog with new version download (installer or app dependent on system) including path selection
-    AppUpdateDownloadDialog dwnld_dlg(*app_data.version);
+    AppUpdateDownloadDialog dwnld_dlg(*app_data.version, app_data.target_path);
     dialog_result = dwnld_dlg.ShowModal();
     //  Doesn't wish to download
     if (dialog_result != wxID_OK) {
         return;
     }
-    // Save as dialog
+    app_data.target_path =dwnld_dlg.get_download_path();
-    if (dwnld_dlg.select_download_path()) {
+
-        std::string extension = app_data.target_path.filename().extension().string();
-        wxString wildcard;
-        if (!extension.empty()) {
-            extension = extension.substr(1);
-            wxString wxext = boost::nowide::widen(extension);
-            wildcard = GUI::format_wxstr("%1% Files (*.%2%)|*.%2%", wxext.Upper(), wxext);
-        }
-        wxFileDialog save_dlg(
-            plater()
-            , _L("Save as:")
-            , boost::nowide::widen(m_app_updater->get_default_dest_folder())
-            , boost::nowide::widen(AppUpdater::get_filename_from_url(app_data.url))
-            , wildcard
-            , wxFD_SAVE | wxFD_OVERWRITE_PROMPT
-        );
-        // Canceled
-        if (save_dlg.ShowModal() != wxID_OK) {
-            return;
-        // set path
-        } else {
-            app_data.target_path = boost::filesystem::path(save_dlg.GetPath().ToUTF8().data());
-        }
-    }
-    if (boost::filesystem::exists(app_data.target_path))
-    {
-        //lm:UI confirmation dialog?
-        BOOST_LOG_TRIVIAL(error) << "App download: File on target path already exists and will be overwritten. Path: " << app_data.target_path;
-    }
     // start download
     this->plater_->get_notification_manager()->push_download_progress_notification(_utf8("Download"), std::bind(&AppUpdater::cancel_callback, this->m_app_updater.get()));
     app_data.start_after = dwnld_dlg.run_after_download();
@@ -3365,10 +3309,17 @@ void GUI_App::app_updater(bool from_user)
     m_app_updater->sync_download();
 }
 
-void GUI_App::app_version_check()
+void GUI_App::app_version_check(bool from_user)
 {
+    if (from_user) {
+        if (m_app_updater->get_download_ongoing()) {
+            MessageDialog msgdlg(nullptr, _L("Download of new version is already ongoing. Do you wish to continue?"), _L("Notice"), wxYES_NO);
+            if (msgdlg.ShowModal() != wxID_YES)
+                return;
+        }
+    }
     std::string version_check_url = app_config->version_check_url();
-    m_app_updater->sync_version(version_check_url);
+    m_app_updater->sync_version(version_check_url, from_user);
 }
 
 } // GUI

src/slic3r/GUI/GUI_App.hpp

@@ -367,7 +367,7 @@ private:
     // if the data from version file are already downloaded, shows dialogs to start download of new version of app
     void            app_updater(bool from_user);
     // inititate read of version file online in separate thread
-    void            app_version_check();
+    void            app_version_check(bool from_user);
 
     bool            m_datadir_redefined { false }; 
 };

src/slic3r/GUI/NotificationManager.cpp

@@ -1787,7 +1787,12 @@ void NotificationManager::set_download_progress_percentage(float percentage)
 {
  	for (std::unique_ptr<PopNotification>& notification : m_pop_notifications) {
 		if (notification->get_type() == NotificationType::AppDownload) {
-			dynamic_cast<ProgressBarWithCancelNotification*>(notification.get())->set_percentage(percentage);
+			ProgressBarWithCancelNotification* pbwcn = dynamic_cast<ProgressBarWithCancelNotification*>(notification.get());
+			// if this changes the percentage, it should be shown now
+			float percent_b4 = pbwcn->get_percentage();
+			pbwcn->set_percentage(percentage);
+			if (pbwcn->get_percentage() != percent_b4)
+				wxGetApp().plater()->get_current_canvas3D()->schedule_extra_frame(0);
 			return;
 		}
 	}

src/slic3r/GUI/NotificationManager.hpp

@@ -442,6 +442,7 @@ private:
 		
 		ProgressBarNotification(const NotificationData& n, NotificationIDProvider& id_provider, wxEvtHandler* evt_handler) : PopNotification(n, id_provider, evt_handler) { }
 		virtual void set_percentage(float percent) { m_percentage = percent; }
+		float get_percentage() const { return m_percentage; }
 	protected:
 		virtual void init() override;		
 		virtual void	render_text(ImGuiWrapper& imgui,
@@ -474,6 +475,7 @@ private:
 		}
 		void	set_percentage(float percent) override { m_percentage = percent; if(m_percentage >= 1.f) m_state = EState::FadingOut; else m_state = EState::NotFading; }
 		void	set_cancel_callback(std::function<bool()> cancel_callback) { m_cancel_callback = cancel_callback; }
+
 	protected:
 		void	render_close_button(ImGuiWrapper& imgui,
 										const float win_size_x, const float win_size_y,

src/slic3r/GUI/UpdateDialogs.cpp

@@ -14,11 +14,13 @@
 
 #include "libslic3r/libslic3r.h"
 #include "libslic3r/Utils.hpp"
+#include "../Utils/AppUpdater.hpp"
 #include "GUI.hpp"
 #include "GUI_App.hpp"
 #include "I18N.hpp"
 #include "ConfigWizard.hpp"
 #include "wxExtensions.hpp"
+#include "format.hpp"
 
 namespace Slic3r {
 namespace GUI {
@@ -126,23 +128,50 @@ bool AppUpdateAvailableDialog::disable_version_check() const
 }
 
 // AppUpdateDownloadDialog
-AppUpdateDownloadDialog::AppUpdateDownloadDialog( const Semver& ver_online)
+AppUpdateDownloadDialog::AppUpdateDownloadDialog( const Semver& ver_online, boost::filesystem::path& path)
 	: MsgDialog(nullptr, _(L("App Update download")), wxString::Format(_(L("New version of %s is available.")), SLIC3R_APP_NAME))
 {
-
 	auto* versions = new wxFlexGridSizer(2, 0, VERT_SPACING);
 	versions->Add(new wxStaticText(this, wxID_ANY, _(L("New version:"))));
 	versions->Add(new wxStaticText(this, wxID_ANY, ver_online.to_string()));
 	content_sizer->Add(versions);
 	content_sizer->AddSpacer(VERT_SPACING);
 #ifndef __linux__
-	cbox_run = new wxCheckBox(this, wxID_ANY, _(L("Run installer after download.")));
+	cbox_run = new wxCheckBox(this, wxID_ANY, _(L("Run installer after download. (Otherwise file explorer will be opened)")));
 	content_sizer->Add(cbox_run);
 #endif
 	content_sizer->AddSpacer(VERT_SPACING);
-	cbox_path = new wxCheckBox(this, wxID_ANY, _(L("Select path for downloaded file.")));
-	content_sizer->Add(cbox_path);
 	content_sizer->AddSpacer(VERT_SPACING);
+	content_sizer->Add(new wxStaticText(this, wxID_ANY, _(L("Target path:"))));
+	content_sizer->AddSpacer(VERT_SPACING);
+	txtctrl_path = new wxTextCtrl(this, wxID_ANY, path.wstring());
+	content_sizer->Add(txtctrl_path, 1, wxEXPAND);
+	content_sizer->AddSpacer(VERT_SPACING);
+	
+	wxButton* btn = new wxButton(this, wxID_ANY, _L("Select path"));
+	content_sizer->Add(btn/*, 1, wxEXPAND*/);
+
+	// button to open file dialog
+	btn->Bind(wxEVT_BUTTON, ([this, path](wxCommandEvent& e) {
+		std::string extension = path.filename().extension().string();
+		wxString wildcard;
+		if (!extension.empty()) {
+			extension = extension.substr(1);
+			wxString wxext = boost::nowide::widen(extension);
+			wildcard = GUI::format_wxstr("%1% Files (*.%2%)|*.%2%", wxext.Upper(), wxext);
+		}
+		wxFileDialog save_dlg(
+			this
+			, _L("Save as:")
+			, txtctrl_path->GetValue()
+			, boost::nowide::widen(AppUpdater::get_filename_from_url(txtctrl_path->GetValue().ToUTF8().data()))
+			, wildcard
+			, wxFD_SAVE | wxFD_OVERWRITE_PROMPT
+		);
+		if (save_dlg.ShowModal() == wxID_OK) {
+			txtctrl_path->SetValue(save_dlg.GetPath());
+		}
+	}));
 
 	content_sizer->SetMinSize(AppUpdateAvailableDialog::AUAD_size);
 
@@ -150,7 +179,16 @@ AppUpdateDownloadDialog::AppUpdateDownloadDialog( const Semver& ver_online)
 
 	if (auto* btn_ok = get_button(wxID_OK); btn_ok != NULL) {
 		btn_ok->SetLabel(_L("Download"));
+		btn_ok->Bind(wxEVT_BUTTON, ([this, path](wxCommandEvent& e){
+			if (boost::filesystem::exists(boost::filesystem::path(txtctrl_path->GetValue().ToUTF8().data()))) {
+				MessageDialog msgdlg(nullptr, GUI::format_wxstr(_L("File %1% already exists. Do you wish to overwrite it?"), txtctrl_path->GetValue()),_L("Notice"), wxYES_NO);
+				if (msgdlg.ShowModal() != wxID_YES)
+					return;
 			}
+			this->EndModal(wxID_OK);
+		}));
+	}
+
 
 	finalize();
 }
@@ -166,9 +204,9 @@ bool AppUpdateDownloadDialog::run_after_download() const
 	return false;
 }
 
-bool AppUpdateDownloadDialog::select_download_path() const
+boost::filesystem::path AppUpdateDownloadDialog::get_download_path() const
 {
-	return cbox_path->GetValue();
+	return std::move(boost::filesystem::path(txtctrl_path->GetValue().ToUTF8().data()));
 }
 
 // MsgUpdateConfig

src/slic3r/GUI/UpdateDialogs.hpp

@@ -57,7 +57,7 @@ private:
 class AppUpdateDownloadDialog : public MsgDialog
 {
 public:
-	AppUpdateDownloadDialog(const Semver& ver_online);
+	AppUpdateDownloadDialog(const Semver& ver_online, boost::filesystem::path& path);
 	AppUpdateDownloadDialog(AppUpdateDownloadDialog&&) = delete;
 	AppUpdateDownloadDialog(const AppUpdateDownloadDialog&) = delete;
 	AppUpdateDownloadDialog& operator=(AppUpdateDownloadDialog&&) = delete;
@@ -66,11 +66,11 @@ public:
 
 	// Tells whether the user checked the "don't bother me again" checkbox
 	bool		run_after_download() const;
-	bool select_download_path() const;
+	boost::filesystem::path	get_download_path() const;
 
 private:
 	wxCheckBox* cbox_run;
-	wxCheckBox* cbox_path;
+	wxTextCtrl* txtctrl_path;
 };
 
 // Confirmation dialog informing about configuration update. Lists updated bundles & their versions.

src/slic3r/Utils/AppUpdater.cpp

@@ -14,6 +14,8 @@
 #include "slic3r/GUI/GUI.hpp"
 #include "slic3r/Utils/Http.hpp"
 
+#include "libslic3r/Utils.hpp"
+
 #ifdef _WIN32
 #include <shellapi.h>
 #include <Shlobj_core.h>
@@ -30,44 +32,16 @@ namespace {
 #ifdef _WIN32
 	bool run_file(const boost::filesystem::path& path)
 	{
-		// find updater exe
+		std::string msg;
-		if (boost::filesystem::exists(path)) {
+		bool res = GUI::create_process(path, std::wstring(), msg);
-			// run updater. Original args: /silent -restartapp prusa-slicer.exe -startappfirst
+		if (!res) {
-
+			std::string full_message = GUI::format("Running downloaded instaler of %1% has failed:\n%2%", SLIC3R_APP_NAME, msg);
-			// Using quoted string as mentioned in CreateProcessW docs, silent execution parameter.
+			BOOST_LOG_TRIVIAL(error) << full_message; // lm: maybe UI error msg?  // dk: bellow. (maybe some general show error evt would be better?)
-			std::wstring wcmd = L"\"" + path.wstring(); //lm: closing quote?
+			wxCommandEvent* evt = new wxCommandEvent(EVT_SLIC3R_APP_DOWNLOAD_FAILED);
-
+			evt->SetString(full_message);
-			// additional information
+			GUI::wxGetApp().QueueEvent(evt);
-			STARTUPINFOW si;
-			PROCESS_INFORMATION pi;
-
-			// set the size of the structures
-			ZeroMemory(&si, sizeof(si));
-			si.cb = sizeof(si);
-			ZeroMemory(&pi, sizeof(pi));
-
-			// start the program up
-			if (CreateProcessW(NULL,   // the path
-				wcmd.data(),    // Command line
-				NULL,           // Process handle not inheritable
-				NULL,           // Thread handle not inheritable
-				FALSE,          // Set handle inheritance to FALSE
-				0,              // No creation flags
-				NULL,           // Use parent's environment block
-				NULL,           // Use parent's starting directory 
-				&si,            // Pointer to STARTUPINFO structure
-				&pi             // Pointer to PROCESS_INFORMATION structure (removed extra parentheses)
-			)) {
-				// Close process and thread handles.
-				CloseHandle(pi.hProcess);
-				CloseHandle(pi.hThread);
-				return true;
 		}
-			else {
+		return res;
-				BOOST_LOG_TRIVIAL(error) << "Failed to run " << wcmd; //lm: maybe a UI error message?
-			}
-		}
-		return false;
 	}
 
 	std::string get_downloads_path()
@@ -81,108 +55,6 @@ namespace {
 		CoTaskMemFree(path);
 		return ret;
 	}
-
-	bool open_folder(const boost::filesystem::path& path)
-	{
-		// this command can run the installer exe as well, but is it better than CreateProcessW?
-		ShellExecuteW(NULL, NULL, path.wstring().c_str(), NULL, NULL, SW_SHOWNORMAL);
-		//lm:I would make it explicit that the folder should be opened.
-		//lm:Also, this always returns true.
-		return true;
-	}
-
-#elif __linux__
-	bool run_file(const boost::filesystem::path& path)
-	{	
-		return false;
-	}
-
-	std::string get_downloads_path()
-	{
-		wxString command = "xdg-user-dir DOWNLOAD";
-		wxArrayString output;
-		 
-		 //lm:It might be a good idea to make the following a separate function in GUI_Utils or something
-		 // It is already used in four different places in almost the same way.
-
-		//Check if we're running in an AppImage container, if so, we need to remove AppImage's env vars,
-		// because they may mess up the environment expected by the file manager.
-		// Mostly this is about LD_LIBRARY_PATH, but we remove a few more too for good measure.
-		if (wxGetEnv("APPIMAGE", nullptr)) {
-			// We're running from AppImage
-			wxEnvVariableHashMap env_vars;
-			wxGetEnvMap(&env_vars);
-
-			env_vars.erase("APPIMAGE");
-			env_vars.erase("APPDIR");
-			env_vars.erase("LD_LIBRARY_PATH");
-			env_vars.erase("LD_PRELOAD");
-			env_vars.erase("UNION_PRELOAD");
-
-			wxExecuteEnv exec_env;
-			exec_env.env = std::move(env_vars);
-
-			wxString owd;
-			if (wxGetEnv("OWD", &owd)) {
-				// This is the original work directory from which the AppImage image was run,
-				// set it as CWD for the child process:
-					exec_env.cwd = std::move(owd);
-			}
-
-			::wxExecute(command, output, 0, &exec_env);
-				
-		} else {
-			// Looks like we're NOT running from AppImage, we'll make no changes to the environment.
-			::wxExecute(command, output);
-		}
-		if (output.GetCount() > 0) {
-			return boost::nowide::narrow(output[0]); //lm:I would use wxString::ToUTF8(), although on Linux, nothing at all should work too.
-		}
-		return std::string();
-	}
-
-	bool open_folder(const boost::filesystem::path& path)
-	{
-		if (boost::filesystem::is_directory(path)) {
-			const char *argv[] = { "xdg-open", path.string().c_str(), nullptr };
-
-			//lm:This is a copy of desktop_open_datadir_folder, it would make sense to instead call it.
-
-			// Check if we're running in an AppImage container, if so, we need to remove AppImage's env vars,
-			// because they may mess up the environment expected by the file manager.
-			// Mostly this is about LD_LIBRARY_PATH, but we remove a few more too for good measure.
-			if (wxGetEnv("APPIMAGE", nullptr)) {
-				// We're running from AppImage
-				wxEnvVariableHashMap env_vars;
-				wxGetEnvMap(&env_vars);
-
-				env_vars.erase("APPIMAGE");
-				env_vars.erase("APPDIR");
-				env_vars.erase("LD_LIBRARY_PATH");
-				env_vars.erase("LD_PRELOAD");
-				env_vars.erase("UNION_PRELOAD");
-
-				wxExecuteEnv exec_env;
-				exec_env.env = std::move(env_vars);
-
-				wxString owd;
-				if (wxGetEnv("OWD", &owd)) {
-					// This is the original work directory from which the AppImage image was run,
-					// set it as CWD for the child process:
-					exec_env.cwd = std::move(owd);
-				}
-
-				::wxExecute(const_cast<char**>(argv), wxEXEC_ASYNC, nullptr, &exec_env);
-
-			} else {
-				// Looks like we're NOT running from AppImage, we'll make no changes to the environment.
-				::wxExecute(const_cast<char**>(argv), wxEXEC_ASYNC, nullptr, nullptr);
-			}
-			return true;
-		}
-		return false;
-	}
-
 #elif  __APPLE__
 	bool run_file(const boost::filesystem::path& path)
 	{
@@ -203,24 +75,30 @@ namespace {
 		// call objective-c implementation
 		return get_downloads_path_mac();
 	}
-
+#else
-	bool open_folder(const boost::filesystem::path& path)
+	bool run_file(const boost::filesystem::path& path)
 	{	
-
-		if (boost::filesystem::is_directory(path)) {
-			const char* argv[] = { "open", path.string().c_str(), nullptr };
-			::wxExecute(const_cast<char**>(argv), wxEXEC_ASYNC, nullptr);
-			return true;
-		}
 		return false;
 	}
-#endif 
+
+	std::string get_downloads_path()
+	{
+		wxString command = "xdg-user-dir DOWNLOAD";
+		wxArrayString output;
+		GUI::desktop_execute_get_result(command, output);
+		if (output.GetCount() > 0) {
+			return output[0].ToUTF8().data(); //lm:I would use wxString::ToUTF8(), although on Linux, nothing at all should work too.
+		}
+		return std::string();
+	}
+#endif // _WIN32 / __apple__ / else 
 } // namespace
 
 wxDEFINE_EVENT(EVT_SLIC3R_VERSION_ONLINE, wxCommandEvent);
 wxDEFINE_EVENT(EVT_SLIC3R_EXPERIMENTAL_VERSION_ONLINE, wxCommandEvent);
 wxDEFINE_EVENT(EVT_SLIC3R_APP_DOWNLOAD_PROGRESS, wxCommandEvent);
 wxDEFINE_EVENT(EVT_SLIC3R_APP_DOWNLOAD_FAILED, wxCommandEvent);
+wxDEFINE_EVENT(EVT_SLIC3R_APP_OPEN_FAILED, wxCommandEvent);
 
 // priv handles all operations in separate thread
 // 1) download version file and parse it.
@@ -251,12 +129,19 @@ struct AppUpdater::priv {
 	std::thread				m_thread;
 	std::atomic_bool        m_cancel;
 	std::mutex				m_data_mutex;
+	// used to tell if notify user hes about to stop ongoing download
+	std::atomic_bool		m_download_ongoing { false };
+	bool					get_download_ongoing() const { return m_download_ongoing; }
 	// read only variable used to init m_online_version_data.target_path
 	boost::filesystem::path m_default_dest_folder; // readonly
 	// DownloadAppData read / write needs to be locked by m_data_mutex
 	DownloadAppData			m_online_version_data;
 	DownloadAppData get_app_data();
 	void            set_app_data(DownloadAppData data);
+	// set only before version file is downloaded, to keep information to show info dialog about no updates
+	// should never change during thread run
+	std::atomic_bool		m_triggered_by_user {false};
+	bool					get_triggered_by_user() const { return m_triggered_by_user; }
 };
 
 AppUpdater::priv::priv() :
@@ -271,7 +156,7 @@ AppUpdater::priv::priv() :
 	if (!downloads_path.empty()) {
 		m_default_dest_folder = std::move(downloads_path);
 	}
-	BOOST_LOG_TRIVIAL(error) << "Default download path: " << m_default_dest_folder; //lm:Is this an error?
+	BOOST_LOG_TRIVIAL(trace) << "App updater default download path: " << m_default_dest_folder; //lm:Is this an error? // dk: changed to trace
 	
 }
 
@@ -284,7 +169,7 @@ bool  AppUpdater::priv::http_get_file(const std::string& url, size_t size_limit,
 			// progress function returns true as success (to continue) 
 			cancel = (this->m_cancel ? true : !progress_fn(std::move(progress)));
 			if (cancel) {
-				error_message = GUI::format("Error getting: `%1%`: Download was canceled.", //lm:typo
+				error_message = GUI::format("Error getting: `%1%`: Download was canceled.", //lm:typo //dk: am i blind? :)
 					url);
 				BOOST_LOG_TRIVIAL(debug) << "AppUpdater::priv::http_get_file message: "<< error_message;
 			}
@@ -318,7 +203,7 @@ boost::filesystem::path AppUpdater::priv::download_file(const DownloadAppData& d
 		return boost::filesystem::path();
 	}
 	std::string error_message;
-	bool res = http_get_file(data.url, 80 * 1024 * 1024 //TODO: what value here //lm:I don't know, but larger. The binaries will grow.
+	bool res = http_get_file(data.url, 130 * 1024 * 1024 //2.4.0 windows installer is 65MB //lm:I don't know, but larger. The binaries will grow. // dk: changed to 130, to have 100% more space. We should put this information into version file. 
 		// on_progress
 		, [&last_gui_progress, expected_size](Http::Progress progress) {
 			// size check
@@ -329,12 +214,13 @@ boost::filesystem::path AppUpdater::priv::download_file(const DownloadAppData& d
 				evt->SetString(message);
 				GUI::wxGetApp().QueueEvent(evt);
 				return false;
-			} else if (progress.dltotal > 0 && progress.dltotal < expected_size) { //lm:When will this happen? Is that not an error?
+			} else if (progress.dltotal > 0 && progress.dltotal < expected_size) { 
-				BOOST_LOG_TRIVIAL(error) << GUI::format("Downloading new %1% has incorrect size. The download will continue. \nExpected size: %2%\nDownload size: %3%", SLIC3R_APP_NAME, expected_size, progress.dltotal);;
+				//lm:When will this happen? Is that not an error? // dk: It is possible error, but we cannot know until the download is finished. Somehow the total size can grow during the download.
+				BOOST_LOG_TRIVIAL(info) << GUI::format("Downloading new %1% has incorrect size. The download will continue. \nExpected size: %2%\nDownload size: %3%", SLIC3R_APP_NAME, expected_size, progress.dltotal);
 			} 
 			// progress event
 			size_t gui_progress = progress.dltotal > 0 ? 100 * progress.dlnow / progress.dltotal : 0;
-			//BOOST_LOG_TRIVIAL(error) << "App download " << gui_progress << "% " << progress.dlnow << " of " << progress.dltotal;
+			BOOST_LOG_TRIVIAL(error) << "App download " << gui_progress << "% " << progress.dlnow << " of " << progress.dltotal;
 			if (last_gui_progress < gui_progress && (last_gui_progress != 0 || gui_progress != 100)) {
 				last_gui_progress = gui_progress;
 				wxCommandEvent* evt = new wxCommandEvent(EVT_SLIC3R_APP_DOWNLOAD_PROGRESS);
@@ -348,8 +234,8 @@ boost::filesystem::path AppUpdater::priv::download_file(const DownloadAppData& d
 			// Size check. Does always 1 char == 1 byte?
 			size_t body_size = body.size(); 
 			if (body_size != expected_size) {
-				//lm:UI message?
+				//lm:UI message? // dk: changed. Now it propagates to UI.
-				BOOST_LOG_TRIVIAL(error) << "Downloaded file has wrong size. Expected size: " <<  expected_size << " Downloaded size: " << body_size;
+				error_message = GUI::format("Downloaded file has wrong size. Expected size: %1% Downloaded size: %2%", expected_size, body_size);
 				return false;
 			}
 			boost::filesystem::path tmp_path = dest_path;
@@ -363,7 +249,7 @@ boost::filesystem::path AppUpdater::priv::download_file(const DownloadAppData& d
 			}
 			catch (const std::exception&)
 			{
-				BOOST_LOG_TRIVIAL(error) << "Failed to write and move " << tmp_path << " to " << dest_path;
+				error_message = GUI::format("Failed to write and move %1% to %2%", tmp_path, dest_path);
 				return false;
 			}
 			return true;
@@ -374,7 +260,9 @@ boost::filesystem::path AppUpdater::priv::download_file(const DownloadAppData& d
 	{
 		if (this->m_cancel)
 		{
-			BOOST_LOG_TRIVIAL(error) << error_message; //lm:Is this an error?
+			BOOST_LOG_TRIVIAL(info) << error_message; //lm:Is this an error? // dk: changed to info
+			wxCommandEvent* evt = new wxCommandEvent(EVT_SLIC3R_APP_DOWNLOAD_FAILED); // FAILED with empty msg only closes progress notification
+			GUI::wxGetApp().QueueEvent(evt);
 		} else {
 			std::string message = GUI::format("Downloading new %1% has failed:\n%2%", SLIC3R_APP_NAME, error_message);
 			BOOST_LOG_TRIVIAL(error) << message;
@@ -391,9 +279,7 @@ boost::filesystem::path AppUpdater::priv::download_file(const DownloadAppData& d
 bool AppUpdater::priv::run_downloaded_file(boost::filesystem::path path)
 {
 	assert(!path.empty());
-	bool res = run_file(path);
+	return run_file(path);
-	BOOST_LOG_TRIVIAL(error) << "Running "<< path.string() << " was " << res;
-	return res;
 }
 
 void AppUpdater::priv::version_check(const std::string& version_check_url) 
@@ -414,8 +300,16 @@ void AppUpdater::priv::version_check(const std::string& version_check_url)
 	//lm:In case the internet is not available, it will report no updates if run by user.
 	// We might save a flag that we don't know or try to run the version_check again, reporting
 	// the failure.
-	if (!res)
+	// dk: changed to download version every time. Dialog will show if m_triggered_by_user.
-		BOOST_LOG_TRIVIAL(error) << "Failed to download version file: " << error_message;
+	if (!res) {
+		std::string message = GUI::format("Downloading %1% version file has failed:\n%2%", SLIC3R_APP_NAME, error_message);
+		BOOST_LOG_TRIVIAL(error) << message;
+		if (m_triggered_by_user) {
+			wxCommandEvent* evt = new wxCommandEvent(EVT_SLIC3R_APP_DOWNLOAD_FAILED);
+			evt->SetString(message);
+			GUI::wxGetApp().QueueEvent(evt);
+		}
+	}
 }
 
 void AppUpdater::priv::parse_version_string(const std::string& body)
@@ -450,13 +344,14 @@ void AppUpdater::priv::parse_version_string(const std::string& body)
 		if (section_name == 
 #ifdef _WIN32
 			"release:win64"
-#elif __linux__
+#elif __APPLE__
-			"release:linux"
-#else
 			"release:osx"
+#else
+			"release:linux"
 #endif
 //lm:Related to the ifdefs. We should also support BSD, which behaves similar to Linux in most cases.
 // Unless you have a reason not to, I would consider doing _WIN32, elif __APPLE__, else ... Not just here.
+// dk: so its ok now or we need to specify BSD?
 			) {
 			for (const auto& data : section.second) {
 				if (data.first == "url") {
@@ -530,7 +425,7 @@ void AppUpdater::priv::parse_version_string(const std::string& body)
 	GUI::wxGetApp().QueueEvent(evt);
 }
 
-#if 0 //lm:is this meant to be ressurected?
+#if 0 //lm:is this meant to be ressurected? //dk: it is code that parses PrusaSlicer.version2 in 2.4.0, It was deleted from PresetUpdater.cpp and I would keep it here for possible reference.
 void AppUpdater::priv::parse_version_string_old(const std::string& body) const
 {
 
@@ -643,17 +538,19 @@ void AppUpdater::sync_download()
 
  	p->m_thread = std::thread(
 		[this, input_data]() {
+			p->m_download_ongoing = true;
 			if (boost::filesystem::path dest_path = p->download_file(input_data); boost::filesystem::exists(dest_path)){
 				if (input_data.start_after) {
 					p->run_downloaded_file(std::move(dest_path));
 				} else {
-					open_folder(dest_path.parent_path());
+					GUI::desktop_open_folder(dest_path.parent_path());
 				}
 			}
+			p->m_download_ongoing = false;
 		});
 }
 
-void AppUpdater::sync_version(const std::string& version_check_url)
+void AppUpdater::sync_version(const std::string& version_check_url, bool from_user)
 {
 	assert(p);
 	// join thread first - it could have been in sync_download
@@ -663,6 +560,7 @@ void AppUpdater::sync_version(const std::string& version_check_url)
 		p->m_cancel = true;
 		p->m_thread.join();
 	}
+	p->m_triggered_by_user = from_user;
 	p->m_cancel = false;
 	p->m_thread = std::thread(
 		[this, version_check_url]() {
@@ -707,5 +605,14 @@ DownloadAppData AppUpdater::get_app_data()
 	return p->get_app_data();
 }
 
+bool AppUpdater::get_triggered_by_user() const
+{
+	return p->get_triggered_by_user();
+}
+
+bool AppUpdater::get_download_ongoing() const
+{
+	return p->get_download_ongoing();
+}
 
 } //namespace Slic3r 

src/slic3r/Utils/AppUpdater.hpp

@@ -37,7 +37,7 @@ public:
 	// downloads app file
 	void sync_download();
 	// downloads version file
-	void sync_version(const std::string& version_check_url);
+	void sync_version(const std::string& version_check_url, bool from_user);
 	void cancel();
 	bool cancel_callback();
 
@@ -46,6 +46,9 @@ public:
 	static std::string get_filename_from_url(const std::string& url);
 	static std::string get_file_extension_from_url(const std::string& url);
 
+	// atomic bool
+	bool				get_triggered_by_user() const;
+	bool				get_download_ongoing() const;
 	// mutex access
 	void				set_app_data(DownloadAppData data);
 	DownloadAppData		get_app_data();
@@ -58,5 +61,6 @@ wxDECLARE_EVENT(EVT_SLIC3R_VERSION_ONLINE, wxCommandEvent);
 wxDECLARE_EVENT(EVT_SLIC3R_EXPERIMENTAL_VERSION_ONLINE, wxCommandEvent);
 wxDECLARE_EVENT(EVT_SLIC3R_APP_DOWNLOAD_PROGRESS, wxCommandEvent);
 wxDECLARE_EVENT(EVT_SLIC3R_APP_DOWNLOAD_FAILED, wxCommandEvent);
+wxDECLARE_EVENT(EVT_SLIC3R_APP_OPEN_FAILED, wxCommandEvent);
 } //namespace Slic3r 
 #endif

src/slic3r/Utils/PresetUpdater.cpp

@@ -135,10 +135,6 @@ struct Updates
 	std::vector<Update> updates;
 };
 
-
-//wxDEFINE_EVENT(EVT_SLIC3R_VERSION_ONLINE, wxCommandEvent);
-//wxDEFINE_EVENT(EVT_SLIC3R_EXPERIMENTAL_VERSION_ONLINE, wxCommandEvent);
-
 struct PresetUpdater::priv
 {
 	std::vector<Index> index_db;
@@ -162,8 +158,6 @@ struct PresetUpdater::priv
 	void set_download_prefs(AppConfig *app_config);
 	bool get_file(const std::string &url, const fs::path &target_path) const;
 	void prune_tmps() const;
-//	void sync_version() const;
-//	void parse_version_string(const std::string& body) const;
 	void sync_config(const VendorMap vendors);
 
 	void check_install_indices() const;
@@ -238,105 +232,6 @@ void PresetUpdater::priv::prune_tmps() const
 		}
 }
 
-// moved to app updater
-	/*
-// Get Slic3rPE version available online, save in AppConfig.
-void PresetUpdater::priv::sync_version() const
-{
-	if (! enabled_version_check) { return; }
-
-	BOOST_LOG_TRIVIAL(info) << format("Downloading %1% online version from: `%2%`", SLIC3R_APP_NAME, version_check_url);
-
-	Http::get(version_check_url)
-		.size_limit(SLIC3R_VERSION_BODY_MAX)
-		.on_progress([this](Http::Progress, bool &cancel) {
-			cancel = this->cancel;
-		})
-		.on_error([&](std::string body, std::string error, unsigned http_status) {
-			(void)body;
-			BOOST_LOG_TRIVIAL(error) << format("Error getting: `%1%`: HTTP %2%, %3%",
-				version_check_url,
-				http_status,
-				error);
-		})
-		.on_complete([&](std::string body, unsigned ) {
-			boost::trim(body);
-			parse_version_string(body);
-		})
-		.perform_sync();
-}
-
-// Parses version string obtained in sync_version() and sends events to UI thread.
-// Version string must contain release version on first line. Follows non-mandatory alpha / beta releases on following lines (alpha=2.0.0-alpha1).
-void PresetUpdater::priv::parse_version_string(const std::string& body) const
-{
-	
-	// release version
-	std::string version;
-	const auto first_nl_pos = body.find_first_of("\n\r");
-	if (first_nl_pos != std::string::npos)
-		version = body.substr(0, first_nl_pos);
-	else
-		version = body;
-	boost::optional<Semver> release_version = Semver::parse(version);
-	if (!release_version) {
-		BOOST_LOG_TRIVIAL(error) << format("Received invalid contents from `%1%`: Not a correct semver: `%2%`", SLIC3R_APP_NAME, version);
-		return;
-	}
-	BOOST_LOG_TRIVIAL(info) << format("Got %1% online version: `%2%`. Sending to GUI thread...", SLIC3R_APP_NAME, version);
-	wxCommandEvent* evt = new wxCommandEvent(EVT_SLIC3R_VERSION_ONLINE);
-	evt->SetString(GUI::from_u8(version));
-	GUI::wxGetApp().QueueEvent(evt);
-
-	// alpha / beta version
-	std::vector<std::string> prerelease_versions;
-	size_t nexn_nl_pos = first_nl_pos;
-	while (nexn_nl_pos != std::string::npos && body.size() > nexn_nl_pos + 1) {
-		const auto last_nl_pos = nexn_nl_pos;
-		nexn_nl_pos = body.find_first_of("\n\r", last_nl_pos + 1);
-		std::string line;
-		if (nexn_nl_pos == std::string::npos)
-			line = body.substr(last_nl_pos + 1);
-		else
-			line = body.substr(last_nl_pos + 1, nexn_nl_pos - last_nl_pos - 1);
-
-		// alpha
-		if (line.substr(0, 6) == "alpha=") {
-			version = line.substr(6);
-			if (!Semver::parse(version)) {
-				BOOST_LOG_TRIVIAL(error) << format("Received invalid contents for alpha release from `%1%`: Not a correct semver: `%2%`", SLIC3R_APP_NAME, version);
-				return;
-			}
-			prerelease_versions.emplace_back(version);
-		// beta
-		}
-		else if (line.substr(0, 5) == "beta=") {
-			version = line.substr(5);
-			if (!Semver::parse(version)) {
-				BOOST_LOG_TRIVIAL(error) << format("Received invalid contents for beta release from `%1%`: Not a correct semver: `%2%`", SLIC3R_APP_NAME, version);
-				return;
-			}
-			prerelease_versions.emplace_back(version);
-		}
-	}
-	// find recent version that is newer than last full release.
-	boost::optional<Semver> recent_version;
-	for (const std::string& ver_string : prerelease_versions) {
-		boost::optional<Semver> ver = Semver::parse(ver_string);
-		if (ver && *release_version < *ver && ((recent_version && *recent_version < *ver) || !recent_version)) {
-			recent_version = ver;
-			version = ver_string;
-		}
-	}
-	if (recent_version) {
-		BOOST_LOG_TRIVIAL(info) << format("Got %1% online version: `%2%`. Sending to GUI thread...", SLIC3R_APP_NAME, version);
-		wxCommandEvent* evt = new wxCommandEvent(EVT_SLIC3R_EXPERIMENTAL_VERSION_ONLINE);
-		evt->SetString(GUI::from_u8(version));
-		GUI::wxGetApp().QueueEvent(evt);
-	}
-	
-}
-*/
 // Download vendor indices. Also download new bundles if an index indicates there's a new one available.
 // Both are saved in cache.
 void PresetUpdater::priv::sync_config(const VendorMap vendors)
@@ -747,7 +642,6 @@ void PresetUpdater::sync(PresetBundle *preset_bundle)
 
     p->thread = std::thread([this, vendors]() {
 		this->p->prune_tmps();
-//		this->p->sync_version();
 		this->p->sync_config(std::move(vendors));
     });
 }