Преглед изворни кода

Run: Don't wait for child process to end when adding it to history

When we try to start a long-running child process (e.g. a GUI app)
using a combination of the POSIX spawn and waitpid API, the Run
process ends up waiting for it to end before making any changes
to the path history. This leads to some confusion when trying to
fire up another Run process only to see that it did not save the
path to this program.

This PR resolves this by saving the path after it was created using
the POSIX spawn API.
Kemal Zebari пре 1 година
родитељ
комит
8893836b60
2 измењених фајлова са 18 додато и 7 уклоњено
  1. 17 7
      Userland/Applications/Run/RunWindow.cpp
  2. 1 0
      Userland/Applications/Run/RunWindow.h

+ 17 - 7
Userland/Applications/Run/RunWindow.cpp

@@ -96,12 +96,6 @@ void RunWindow::do_run()
     hide();
     hide();
 
 
     if (run_via_launch(run_input) || run_as_command(run_input)) {
     if (run_via_launch(run_input) || run_as_command(run_input)) {
-        // Remove any existing history entry, prepend the successful run string to history and save.
-        m_path_history.remove_all_matching([&](ByteString v) { return v == run_input; });
-        m_path_history.prepend(run_input);
-        // FIXME: Handle failure to save history somehow.
-        (void)save_history();
-
         close();
         close();
         return;
         return;
     }
     }
@@ -120,7 +114,11 @@ bool RunWindow::run_as_command(ByteString const& run_input)
 
 
     pid_t child_pid = maybe_child_pid.release_value();
     pid_t child_pid = maybe_child_pid.release_value();
 
 
-    // Command spawned in child shell. Hide and wait for exit code.
+    // The child shell was able to start. Let's save it to the history immediately so users can see it as the first entry the next time they run this program.
+    prepend_history(run_input);
+    // FIXME: Handle failure to save history somehow.
+    (void)save_history();
+
     int status;
     int status;
     if (waitpid(child_pid, &status, 0) < 0)
     if (waitpid(child_pid, &status, 0) < 0)
         return false;
         return false;
@@ -130,6 +128,8 @@ bool RunWindow::run_as_command(ByteString const& run_input)
 
 
     // 127 is typically the shell indicating command not found. 126 for all other errors.
     // 127 is typically the shell indicating command not found. 126 for all other errors.
     if (child_error == 126 || child_error == 127) {
     if (child_error == 126 || child_error == 127) {
+        // There's an opportunity to remove the history entry here since it failed during its runtime, but other implementations (e.g. Windows 11) don't bother removing the entry.
+        // This makes sense, especially for cases where a user is debugging a failing program.
         return false;
         return false;
     }
     }
 
 
@@ -157,6 +157,10 @@ bool RunWindow::run_via_launch(ByteString const& run_input)
         return false;
         return false;
     }
     }
 
 
+    prepend_history(run_input);
+    // FIXME: Handle failure to save history somehow.
+    (void)save_history();
+
     dbgln("Ran via URL launch.");
     dbgln("Ran via URL launch.");
 
 
     return true;
     return true;
@@ -182,6 +186,12 @@ ErrorOr<void> RunWindow::load_history()
     return {};
     return {};
 }
 }
 
 
+void RunWindow::prepend_history(ByteString const& input)
+{
+    m_path_history.remove_all_matching([&](ByteString const& entry) { return input == entry; });
+    m_path_history.prepend(input);
+}
+
 ErrorOr<void> RunWindow::save_history()
 ErrorOr<void> RunWindow::save_history()
 {
 {
     auto file = TRY(Core::File::open(history_file_path(), Core::File::OpenMode::Write));
     auto file = TRY(Core::File::open(history_file_path(), Core::File::OpenMode::Write));

+ 1 - 0
Userland/Applications/Run/RunWindow.h

@@ -32,6 +32,7 @@ private:
     ByteString history_file_path();
     ByteString history_file_path();
     ErrorOr<void> load_history();
     ErrorOr<void> load_history();
     ErrorOr<void> save_history();
     ErrorOr<void> save_history();
+    void prepend_history(ByteString const& input);
 
 
     Vector<ByteString> m_path_history;
     Vector<ByteString> m_path_history;
     NonnullRefPtr<GUI::ItemListModel<ByteString>> m_path_history_model;
     NonnullRefPtr<GUI::ItemListModel<ByteString>> m_path_history_model;