Parcourir la source

Consistency in handling errors executing local custom command + Error details when opening file or editor fails

Previous behavior
- ExecuteShellAndWait showed error popup by the system (which typically ended below WinSCP window)
- ExecuteShell did not show any error message

Merge implementations of ExecuteShellAndWait and ExecuteShell

Source commit: ed6d4aa4605c9677977923a357b4bb18dc14d8b2
Martin Prikryl il y a 9 ans
Parent
commit
f1206ad85f

+ 11 - 11
source/forms/CustomScpExplorer.cpp

@@ -1763,7 +1763,7 @@ void __fastcall TCustomScpExplorerForm::CustomCommand(TStrings * FileList,
 
     if (!LocalCustomCommand.IsFileCommand(Command))
     {
-      ExecuteShell(LocalCustomCommand.Complete(Command, true));
+      ExecuteShellChecked(LocalCustomCommand.Complete(Command, true));
     }
     // remote files?
     else if ((FCurrentSide == osRemote) || LocalFileCommand)
@@ -1868,11 +1868,11 @@ void __fastcall TCustomScpExplorerForm::CustomCommand(TStrings * FileList,
 
               if (NonBlocking)
               {
-                ExecuteShell(ShellCommand);
+                ExecuteShellChecked(ShellCommand);
               }
               else
               {
-                ExecuteShellAndWait(ShellCommand);
+                ExecuteShellCheckedAndWait(ShellCommand);
               }
             }
             else if (LocalFileCommand)
@@ -1886,7 +1886,7 @@ void __fastcall TCustomScpExplorerForm::CustomCommand(TStrings * FileList,
                   UnicodeString FileName = RemoteFileList->Strings[Index];
                   TLocalCustomCommand CustomCommand(Data,
                     Terminal->CurrentDirectory, DefaultDownloadTargetDirectory(), FileName, LocalFile, L"");
-                  ExecuteShellAndWait(CustomCommand.Complete(Command, true));
+                  ExecuteShellCheckedAndWait(CustomCommand.Complete(Command, true));
                 }
               }
               else if (RemoteFileList->Count == 1)
@@ -1898,7 +1898,7 @@ void __fastcall TCustomScpExplorerForm::CustomCommand(TStrings * FileList,
                   TLocalCustomCommand CustomCommand(
                     Data, Terminal->CurrentDirectory, DefaultDownloadTargetDirectory(),
                     FileName, LocalFileList->Strings[Index], L"");
-                  ExecuteShellAndWait(CustomCommand.Complete(Command, true));
+                  ExecuteShellCheckedAndWait(CustomCommand.Complete(Command, true));
                 }
               }
               else
@@ -1914,7 +1914,7 @@ void __fastcall TCustomScpExplorerForm::CustomCommand(TStrings * FileList,
                   TLocalCustomCommand CustomCommand(
                     Data, Terminal->CurrentDirectory, DefaultDownloadTargetDirectory(),
                     FileName, LocalFileList->Strings[Index], L"");
-                  ExecuteShellAndWait(CustomCommand.Complete(Command, true));
+                  ExecuteShellCheckedAndWait(CustomCommand.Complete(Command, true));
                 }
               }
             }
@@ -1925,7 +1925,7 @@ void __fastcall TCustomScpExplorerForm::CustomCommand(TStrings * FileList,
                 TLocalCustomCommand CustomCommand(Data,
                   Terminal->CurrentDirectory, DefaultDownloadTargetDirectory(),
                   RemoteFileList->Strings[Index], L"", L"");
-                ExecuteShellAndWait(CustomCommand.Complete(Command, true));
+                ExecuteShellCheckedAndWait(CustomCommand.Complete(Command, true));
               }
             }
           }
@@ -2035,7 +2035,7 @@ void __fastcall TCustomScpExplorerForm::CustomCommand(TStrings * FileList,
         TLocalCustomCommand CustomCommand(
           Data, Terminal->CurrentDirectory, DefaultDownloadTargetDirectory(),
           L"", L"", FileList);
-        ExecuteShell(CustomCommand.Complete(Command, true));
+        ExecuteShellChecked(CustomCommand.Complete(Command, true));
       }
       else
       {
@@ -2053,7 +2053,7 @@ void __fastcall TCustomScpExplorerForm::CustomCommand(TStrings * FileList,
             TLocalCustomCommand CustomCommand(
               Data, Terminal->CurrentDirectory, DefaultDownloadTargetDirectory(),
               FileName, L"", L"");
-            ExecuteShellAndWait(CustomCommand.Complete(Command, true));
+            ExecuteShellCheckedAndWait(CustomCommand.Complete(Command, true));
           }
         }
         __finally
@@ -2845,7 +2845,7 @@ void __fastcall TCustomScpExplorerForm::CustomExecuteFile(TOperationSide Side,
       Program = ExpandEnvironmentVariables(Program);
       if (!ExecuteShell(Program, Params, Process))
       {
-        throw Exception(FMTLOAD(EDITOR_ERROR, (Program)));
+        throw EOSExtException(FMTLOAD(EDITOR_ERROR, (Program)));
       }
     }
     else
@@ -2853,7 +2853,7 @@ void __fastcall TCustomScpExplorerForm::CustomExecuteFile(TOperationSide Side,
       DebugAssert(Side == osRemote);
       if (!ExecuteShell(FileName, L"", Process))
       {
-        throw Exception(FMTLOAD(EXECUTE_FILE_ERROR, (FileName)));
+        throw EOSExtException(FMTLOAD(EXECUTE_FILE_ERROR, (FileName)));
       }
     }
 

+ 3 - 2
source/forms/Login.cpp

@@ -2358,11 +2358,12 @@ void __fastcall TLoginDialog::SessionTreeExpanding(TObject * /*Sender*/,
 void __fastcall TLoginDialog::ExecuteTool(const UnicodeString & Name)
 {
   UnicodeString Path;
-  if (!FindTool(Name, Path) ||
-      !ExecuteShell(Path, L""))
+  if (!FindTool(Name, Path))
   {
     throw Exception(FMTLOAD(EXECUTE_APP_ERROR, (Name)));
   }
+
+  ExecuteShellChecked(Path, L"");
 }
 //---------------------------------------------------------------------------
 void __fastcall TLoginDialog::RunPageantActionExecute(TObject * /*Sender*/)

+ 1 - 6
source/forms/ScpCommander.cpp

@@ -1586,12 +1586,7 @@ bool __fastcall TScpCommanderForm::ExecuteCommandLine()
     }
     else
     {
-      UnicodeString Program, Params, Dir;
-      SplitCommand(Command, Program, Params, Dir);
-      if (!ExecuteShell(Program, Params))
-      {
-        throw Exception(FMTLOAD(EXECUTE_APP_ERROR, (Program)));
-      }
+      ExecuteShellChecked(Command);
     }
   }
   return Result;

+ 56 - 60
source/windows/GUITools.cpp

@@ -145,10 +145,7 @@ void __fastcall OpenSessionInPutty(const UnicodeString PuttyPath,
 
     // PuTTY is started in its binary directory to allow relative paths in private key,
     // when opening PuTTY's own stored session.
-    if (!ExecuteShell(Program, PuttyParams, true))
-    {
-      throw Exception(FMTLOAD(EXECUTE_APP_ERROR, (Program)));
-    }
+    ExecuteShellChecked(Program, PuttyParams, true);
   }
   else
   {
@@ -187,91 +184,90 @@ bool __fastcall CopyCommandToClipboard(const UnicodeString & Command)
   return Result;
 }
 //---------------------------------------------------------------------------
-static bool __fastcall CopyShellCommandToClipboard(const UnicodeString & Path, const UnicodeString & Params)
-{
-  return CopyCommandToClipboard(FormatCommand(Path, Params));
-}
-//---------------------------------------------------------------------------
-bool __fastcall ExecuteShell(const UnicodeString Path, const UnicodeString Params, bool ChangeWorkingDirectory)
+static bool __fastcall DoExecuteShell(const UnicodeString Path, const UnicodeString Params,
+  bool ChangeWorkingDirectory, HANDLE * Handle)
 {
-  bool Result = true;
-  if (!CopyShellCommandToClipboard(Path, Params))
+  bool Result = CopyCommandToClipboard(FormatCommand(Path, Params));
+
+  if (!Result)
   {
     UnicodeString Directory = ExtractFilePath(Path);
-    const wchar_t * PDirectory = (ChangeWorkingDirectory ? Directory.c_str() : NULL);
-    Result =
-      ((int)ShellExecute(NULL, L"open", (wchar_t*)Path.data(),
-        (wchar_t*)Params.data(), PDirectory, SW_SHOWNORMAL) > 32);
+
+    TShellExecuteInfoW ExecuteInfo;
+    memset(&ExecuteInfo, 0, sizeof(ExecuteInfo));
+    ExecuteInfo.cbSize = sizeof(ExecuteInfo);
+    ExecuteInfo.fMask =
+      SEE_MASK_FLAG_NO_UI |
+      FLAGMASK((Handle != NULL), SEE_MASK_NOCLOSEPROCESS);
+    ExecuteInfo.hwnd = Application->Handle;
+    ExecuteInfo.lpFile = (wchar_t*)Path.data();
+    ExecuteInfo.lpParameters = (wchar_t*)Params.data();
+    ExecuteInfo.lpDirectory = (ChangeWorkingDirectory ? Directory.c_str() : NULL);
+    ExecuteInfo.nShow = SW_SHOW;
+
+    Result = (ShellExecuteEx(&ExecuteInfo) != 0);
+    if (Result)
+    {
+      if (Handle != NULL)
+      {
+        *Handle = ExecuteInfo.hProcess;
+      }
+    }
   }
   return Result;
 }
 //---------------------------------------------------------------------------
-bool __fastcall ExecuteShell(const UnicodeString Command)
+void __fastcall ExecuteShellChecked(const UnicodeString Path, const UnicodeString Params, bool ChangeWorkingDirectory)
 {
-  UnicodeString Program, Params, Dir;
-  SplitCommand(Command, Program, Params, Dir);
-  return ExecuteShell(Program, Params);
+  if (!DoExecuteShell(Path, Params, ChangeWorkingDirectory, NULL))
+  {
+    throw EOSExtException(FMTLOAD(EXECUTE_APP_ERROR, (Path)));
+  }
 }
 //---------------------------------------------------------------------------
-static bool __fastcall DoExecuteShell(HWND ApplicationHandle, const UnicodeString Path, const UnicodeString Params,
-  HANDLE & Handle)
+void __fastcall ExecuteShellChecked(const UnicodeString Command)
 {
-  bool Result;
-
-  TShellExecuteInfoW ExecuteInfo;
-  memset(&ExecuteInfo, 0, sizeof(ExecuteInfo));
-  ExecuteInfo.cbSize = sizeof(ExecuteInfo);
-  ExecuteInfo.fMask = SEE_MASK_NOCLOSEPROCESS;
-  ExecuteInfo.hwnd = ApplicationHandle;
-  ExecuteInfo.lpFile = (wchar_t*)Path.data();
-  ExecuteInfo.lpParameters = (wchar_t*)Params.data();
-  ExecuteInfo.nShow = SW_SHOW;
-
-  Result = (ShellExecuteEx(&ExecuteInfo) != 0);
-  if (Result)
-  {
-    Handle = ExecuteInfo.hProcess;
-  }
-  return Result;
+  UnicodeString Program, Params, Dir;
+  SplitCommand(Command, Program, Params, Dir);
+  ExecuteShellChecked(Program, Params);
 }
 //---------------------------------------------------------------------------
 bool __fastcall ExecuteShell(const UnicodeString Path, const UnicodeString Params,
   HANDLE & Handle)
 {
-  return DoExecuteShell(Application->Handle, Path, Params, Handle);
+  return DoExecuteShell(Path, Params, false, &Handle);
 }
 //---------------------------------------------------------------------------
-bool __fastcall ExecuteShellAndWait(HWND Handle, const UnicodeString Command,
+void __fastcall ExecuteShellCheckedAndWait(const UnicodeString Command,
   TProcessMessagesEvent ProcessMessages)
 {
   UnicodeString Program, Params, Dir;
   SplitCommand(Command, Program, Params, Dir);
   HANDLE ProcessHandle;
-  bool Result = true;
-  if (!CopyShellCommandToClipboard(Program, Params))
+  bool Result = DoExecuteShell(Program, Params, false, &ProcessHandle);
+  if (!Result)
   {
-    Result = DoExecuteShell(Handle, Program, Params, ProcessHandle);
-
-    if (Result)
+    throw EOSExtException(FMTLOAD(EXECUTE_APP_ERROR, (Program)));
+  }
+  else
+  {
+    if (ProcessMessages != NULL)
     {
-      if (ProcessMessages != NULL)
+      unsigned long WaitResult;
+      do
       {
-        unsigned long WaitResult;
-        do
+        WaitResult = WaitForSingleObject(ProcessHandle, 200);
+        if (WaitResult == WAIT_FAILED)
         {
-          WaitResult = WaitForSingleObject(ProcessHandle, 200);
-          if (WaitResult == WAIT_FAILED)
-          {
-            throw Exception(LoadStr(DOCUMENT_WAIT_ERROR));
-          }
-          ProcessMessages();
+          throw Exception(LoadStr(DOCUMENT_WAIT_ERROR));
         }
-        while (WaitResult == WAIT_TIMEOUT);
-      }
-      else
-      {
-        WaitForSingleObject(ProcessHandle, INFINITE);
+        ProcessMessages();
       }
+      while (WaitResult == WAIT_TIMEOUT);
+    }
+    else
+    {
+      WaitForSingleObject(ProcessHandle, INFINITE);
     }
   }
 }

+ 4 - 4
source/windows/GUITools.h

@@ -20,12 +20,12 @@ typedef void __fastcall (__closure* TProcessMessagesEvent)();
 //---------------------------------------------------------------------------
 bool __fastcall FindFile(UnicodeString & Path);
 bool __fastcall FindTool(const UnicodeString & Name, UnicodeString & Path);
-bool __fastcall ExecuteShell(const UnicodeString Path, const UnicodeString Params, bool ChangeWorkingDirectory = false);
-bool __fastcall ExecuteShell(const UnicodeString Command);
+void __fastcall ExecuteShellChecked(const UnicodeString Path, const UnicodeString Params,
+  bool ChangeWorkingDirectory = false);
+void __fastcall ExecuteShellChecked(const UnicodeString Command);
 bool __fastcall ExecuteShell(const UnicodeString Path, const UnicodeString Params,
   HANDLE & Handle);
-bool __fastcall ExecuteShellAndWait(HWND Handle, const UnicodeString Command,
-  TProcessMessagesEvent ProcessMessages);
+void __fastcall ExecuteShellCheckedAndWait(const UnicodeString Command, TProcessMessagesEvent ProcessMessages);
 bool __fastcall CopyCommandToClipboard(const UnicodeString & Command);
 void __fastcall OpenSessionInPutty(const UnicodeString PuttyPath,
   TSessionData * SessionData);

+ 1 - 4
source/windows/Setup.cpp

@@ -1358,10 +1358,7 @@ void __fastcall TUpdateDownloadThread::UpdateDownloaded()
     Params += L" /OpenGettingStarted";
   }
 
-  if (!ExecuteShell(SetupPath, Params))
-  {
-    throw Exception(FMTLOAD(EXECUTE_APP_ERROR, (SetupPath)));
-  }
+  ExecuteShellChecked(SetupPath, Params);
 
   Configuration->Usage->Inc(L"UpdateRuns");
   TerminateApplication();

+ 3 - 7
source/windows/Tools.cpp

@@ -295,10 +295,9 @@ UnicodeString __fastcall StoreFormSize(TForm * Form)
   return FORMAT(L"%d,%d,%s", (Form->Width, Form->Height, SavePixelsPerInch()));
 }
 //---------------------------------------------------------------------------
-bool __fastcall ExecuteShellAndWait(const UnicodeString Command)
+void __fastcall ExecuteShellCheckedAndWait(const UnicodeString Command)
 {
-  return ExecuteShellAndWait(Application->Handle, Command,
-    &Application->ProcessMessages);
+  ExecuteShellCheckedAndWait(Command, &Application->ProcessMessages);
 }
 //---------------------------------------------------------------------------
 bool __fastcall IsKeyPressed(int VirtualKey)
@@ -324,10 +323,7 @@ void __fastcall ExecuteNewInstance(const UnicodeString & Param)
     Arg = FORMAT(L"\"%s\" %s", (Arg, TProgramParams::FormatSwitch(NEWINSTANCE_SWICH)));
   }
 
-  if (!ExecuteShell(Application->ExeName, Arg))
-  {
-    throw Exception(FMTLOAD(EXECUTE_APP_ERROR, (Application->ExeName)));
-  }
+  ExecuteShellChecked(Application->ExeName, Arg);
 }
 //---------------------------------------------------------------------------
 IShellLink * __fastcall CreateDesktopShortCut(const UnicodeString & Name,

+ 1 - 1
source/windows/Tools.h

@@ -10,7 +10,7 @@
 #include <Vcl.Graphics.hpp>
 //---------------------------------------------------------------------------
 void __fastcall CenterFormOn(TForm * Form, TControl * CenterOn);
-bool __fastcall ExecuteShellAndWait(const UnicodeString Command);
+void __fastcall ExecuteShellCheckedAndWait(const UnicodeString Command);
 bool __fastcall IsKeyPressed(int VirtualKey);
 bool __fastcall UseAlternativeFunction();
 bool __fastcall OpenInNewWindow();