#2314: Add step handling to the GUI

cli/src/main/java/com/devonfw/tools/ide/step/StepImpl.java

@@ -16,7 +16,7 @@
 /**
  * Regular implementation of {@link Step}.
  */
-public final class StepImpl implements Step {
+public class StepImpl implements Step {
 
   private static final Logger LOG = LoggerFactory.getLogger(StepImpl.class);
 

documentation/contributing/README.adoc

@@ -21,3 +21,8 @@ Further, we documented link:junit-testing.adoc[JUnit-testing] and link:integrati
 
 For security mapping in URL updater development, see link:cpe-url-updater.adoc[CPE integration for URL updaters].
 
+== Design
+
+If you work on the dashboard, read link:gui-task-model.adoc[GUI task model] first.
+It explains how the GUI renders progress bars and steps through a single abstraction, why the CLI is left untouched, and which parts of the design are load-bearing.
+

documentation/contributing/gui-task-model.adoc

@@ -0,0 +1,89 @@
+= GUI task model
+
+The dashboard (`gui` module) shows long-running work in two places: the status bar of the main window, and the task overview window.
+This page explains how that is modelled and why, so the design is not accidentally undone by a later change.
+
+== One abstraction for two CLI concepts
+
+The CLI reports progress through two unrelated mechanisms, both owned by `IdeContext`:
+
+* `IdeProgressBar` -- quantitative, bounded, short-lived. Created via `newProgressBar(...)`.
+* `Step` -- qualitative, nested, and carrying an outcome (success or failure). Created via `newStep(...)`.
+
+The GUI has to render both as "something is happening", so it defines a single UI-facing interface, `GuiTask`, with a title, a detail line, a subtitle, a
+progress fraction, a `TaskState` and a `TaskStats` tally.
+`ProgressBarTask` and `GuiStep` both implement it.
+
+They cannot share a base class: `ProgressBarTask` must extend `AbstractIdeProgressBar` and `GuiStep` must extend `StepImpl`, and Java has single inheritance.
+The shared state therefore lives in `GuiTaskModel`, which both own by composition and delegate to.
+This is the reason for the delegation boilerplate in those two classes -- it is not accidental, and replacing it with a common superclass is not possible.
+
+`TaskManager` holds exactly one `ObservableList<GuiTask>`, so the status bar and the task overview need only one rendering path.
+
+== The CLI is not modified
+
+`IdeGuiContext` maintains its own `Step` stack rather than the one inherited from `AbstractIdeContext`, whose field is private.
+It can do so because all three access points -- `newStep(boolean, String, Object...)`, `getCurrentStep()` and `endStep(StepImpl)` -- are public and overridable,
+and because `StepImpl.end()` calls `context.endStep(this)` virtually and exactly once.
+That call is the only reliable "step has ended" hook: a step may end through `success()`, `error()` or `close()`, and `run()` calls `close()` after `success()`,
+so overriding `close()` alone is neither sufficient nor unambiguous.
+Note that a step closed without an explicit outcome is recorded as a *failure*, and the GUI renders that honestly.
+
+== Steps and progress bars behave differently on purpose
+
+A progress bar removes itself from the task list when it closes.
+A step does not: its outcome is the point, so it stays until the user dismisses it with the `x` button.
+
+Only *root* steps become their own task.
+Nested steps feed their root: they contribute to its `TaskStats` chips, they name its subtitle while they run, and they are appended to its sub-task list.
+Because every descendant resolves the same root via `getRoot()`, a grandchild is flattened onto the root rather than nested under its own parent -- the model is
+deliberately only one level deep.
+
+== Why the task overview is a `TreeView`
+
+A root step can be expanded to reveal its sub-steps.
+That could have been done by building sub-rows inside a `ListCell`, but `TreeView` avoids three problems rather than solving them:
+
+[cols="1,2,2"]
+|===
+|Concern |Sub-rows in a `ListCell` |`TreeView`
+
+|Expansion state
+|Must be stored on the task, because cells are recycled; a flag on the cell makes scrolling expand the wrong row
+|`TreeItem.expandedProperty()` -- tree items are never recycled, only cells are
+
+|Observing the sub-step list
+|Every cell attaches and detaches a `ListChangeListener`, with the same discipline as the binding cleanup in `updateItem`
+|Cells never observe lists; the sync happens once per task in `TaskOverviewWindowController`
+
+|Row height
+|The parent cell grows when expanded, so the list has to re-measure
+|Expanding adds rows, not height
+|===
+
+The tree is only ever two levels deep: a hidden root holds the tasks, and each task holds its sub-steps as a flat list.
+Keeping the tree in sync is cheap because the sub-step list is *append-only* -- a sub-step is never removed once started -- so no diffing is required.
+
+== Updates reach the UI through bindings
+
+Everything the UI shows is bound to the properties of the task itself: the cells in `TaskWindowCellFactory` and the status bar in `MainController`.
+The task list reports which tasks exist; what happens *within* a task is observed by binding, not by reading values out of a list listener.
+
+There is exactly one place where a value is pushed instead of derived, and it cannot be otherwise.
+`AbstractIdeProgressBar` holds its progress in a plain `long` field that is mutated on a background worker thread, so there is no observable to bind to, and
+JavaFX properties may only be written on the JavaFX Application Thread.
+`ProgressBarTask.publishProgress` performs that hand-off -- and *coalesces* it: copying reads in 1 KiB chunks, so a large archive reports progress hundreds of
+thousands of times, and posting every one of those to the FX thread floods its event queue and freezes the UI.
+Only the latest value is kept and a single update is scheduled; while it is outstanding, further reports just overwrite that value.
+The UI therefore refreshes as fast as it can drain and no faster.
+This is the same technique `javafx.concurrent.Task.updateProgress()` uses internally.
+
+`TaskWindowTest.rapidProgressUpdatesAreCoalescedWithoutLosingTheFinalValue` pins both halves of that behaviour: the number of FX events stays far below the
+number of reported steps, and the final value is never dropped.
+
+== Concurrency
+
+`GuiStateManager.newRunContext()` hands every command execution its own `IdeGuiContext`.
+Commands run on their own threads, and a context keeps the current step in a field, so a shared context would let two concurrent commands corrupt each other's
+step hierarchy.
+A per-run context makes that field safe without any locking.

gui/src/main/java/com/devonfw/ide/gui/App.java

@@ -126,7 +126,7 @@ private void reloadMainView() {
 
   private Parent loadMainView() throws IOException {
 
-    FXMLLoader fxmlLoader = new FXMLLoader(App.class.getResource("main-view.fxml"));
+    FXMLLoader fxmlLoader = new FXMLLoader(App.class.getResource("layout/mainview/main-view.fxml"));
     fxmlLoader.setResources(this.nlsService.getResourceBundle());
     fxmlLoader.setController(new MainController(System.getenv(IdeVariables.IDE_ROOT.getName()), guiStateManager, this.nlsService));
     return fxmlLoader.load();

gui/src/main/java/com/devonfw/ide/gui/MainController.java

@@ -7,9 +7,14 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.stream.Stream;
 
 import javafx.application.Platform;
+import javafx.beans.Observable;
+import javafx.beans.binding.Bindings;
+import javafx.beans.binding.BooleanBinding;
 import javafx.collections.ListChangeListener;
+import javafx.collections.ObservableList;
 import javafx.concurrent.Task;
 import javafx.fxml.FXML;
 import javafx.scene.control.Alert.AlertType;
@@ -22,11 +27,13 @@
 import org.slf4j.LoggerFactory;
 
 import com.devonfw.ide.gui.context.GuiStateManager;
+import com.devonfw.ide.gui.context.IdeGuiContext;
 import com.devonfw.ide.gui.context.ProjectManager;
 import com.devonfw.ide.gui.context.TaskManager;
 import com.devonfw.ide.gui.modal.IdeDialog;
 import com.devonfw.ide.gui.nls.NlsService;
-import com.devonfw.ide.gui.progress.ProgressBarTask;
+import com.devonfw.ide.gui.progress.GuiTask;
+import com.devonfw.ide.gui.progress.TaskState;
 import com.devonfw.ide.gui.progress.taskwindow.TaskOverviewWindow;
 
 /**
@@ -70,6 +77,9 @@ public class MainController {
   private ProgressBar statusProgressBar;
   private final double PROGRESSBAR_VISIBLE_WIDTH = 150.0;
 
+  /** Styling of the status label while it links to the {@link TaskOverviewWindow}. */
+  private static final String STATUS_LINK_STYLE = "-fx-text-fill: blue;-fx-cursor: hand";
+
   private final String ideRootPath;
   private Path projectValue;
   private Path workspaceValue;
@@ -95,42 +105,88 @@ public MainController(String ideRoot, GuiStateManager guiStateManager, NlsServic
     this.languageMap = new LinkedHashMap<>();
     this.nlsService = nlsService;
 
-    setUpTaskListListener();
   }
 
-  private void setUpTaskListListener() {
-
-    ListChangeListener<ProgressBarTask> taskListChangeListener = change -> {
-      List<ProgressBarTask> tasks = taskManager.getTasks();
+  @FXML
+  private void initialize() {
 
+    setProjectsComboBox();
+    initLanguageComboBox();
+    // Rebuild only when the set of tasks changes. Everything that changes within a task reaches the UI through the bindings built below, so reacting to
+    // anything else here would rebuild them for no reason on every progress tick.
+    taskManager.getTasks().addListener((ListChangeListener<GuiTask>) change -> {
       while (change.next()) {
-        if (change.wasAdded()) {
-          LOG.debug("Added: {}", change.getAddedSubList());
-
-          for (ProgressBarTask progressTask : change.getAddedSubList()) {
-            progressTask.currentProgressProperty().addListener((_, _, _) ->
-                updateStatusLabel(tasks)
-            );
-          }
-          updateStatusLabel(tasks);
-        } else if (change.wasRemoved()) {
-          LOG.debug("Removed: {}", change.getRemoved());
-
-          updateStatusLabel(tasks);
-        } else if (change.wasUpdated()) {
-
-          updateStatusLabel(tasks);
+        if (change.wasAdded() || change.wasRemoved()) {
+          bindStatusBar();
+          return;
         }
       }
-    };
-    taskManager.getTasks().addListener(taskListChangeListener);
+    });
+    bindStatusBar();
   }
 
-  @FXML
-  private void initialize() {
+  /**
+   * Binds the status bar to the tasks it reports on.
+   * <p>
+   * The bindings depend on the properties of the tasks themselves, not on the task list, so a task progressing or finishing updates the status bar directly.
+   * Only adding or removing a task changes which properties matter, which is why this is rebuilt on structural changes of the list.
+   */
+  private void bindStatusBar() {
+
+    statusLabel.textProperty().unbind();
+    statusLabel.underlineProperty().unbind();
+    statusLabel.styleProperty().unbind();
+    statusProgressBar.progressProperty().unbind();
+    statusProgressBar.visibleProperty().unbind();
+    statusProgressBar.prefWidthProperty().unbind();
+
+    ObservableList<GuiTask> tasks = taskManager.getTasks();
+    Observable[] dependencies = Stream.concat(
+        tasks.stream().flatMap(task -> Stream.of(task.stateProperty(), task.displayTextProperty(), task.progressProperty())),
+        Stream.of(tasks)).toArray(Observable[]::new);
+
+    BooleanBinding singleTaskRunning = Bindings.createBooleanBinding(() -> getRunningTasks().size() == 1, dependencies);
+    // Whenever there is any task at all the label links to the overview, so finished ones can always be reached and dismissed.
+    BooleanBinding isLink = Bindings.createBooleanBinding(() -> !tasks.isEmpty(), dependencies);
+
+    statusLabel.textProperty().bind(Bindings.createStringBinding(this::buildStatusText, dependencies));
+    statusLabel.underlineProperty().bind(isLink);
+    statusLabel.styleProperty().bind(Bindings.when(isLink).then(STATUS_LINK_STYLE).otherwise(""));
+    statusLabel.setOnMouseClicked(_ -> TaskOverviewWindow.getInstance(taskManager).showRelativeToReferenceNode(statusLabel));
+
+    // a value of GuiTaskModel.INDETERMINATE maps directly onto the indeterminate animation of the JavaFX progress bar.
+    statusProgressBar.progressProperty().bind(Bindings.createDoubleBinding(this::buildStatusProgress, dependencies));
+    statusProgressBar.visibleProperty().bind(singleTaskRunning);
+    statusProgressBar.prefWidthProperty().bind(Bindings.when(singleTaskRunning).then(PROGRESSBAR_VISIBLE_WIDTH).otherwise(0.0));
+  }
 
-    setProjectsComboBox();
-    initLanguageComboBox();
+  /**
+   * @return the tasks that are still running. Finished steps stay in the list until dismissed, so the status bar reports on the running ones.
+   */
+  private List<GuiTask> getRunningTasks() {
+
+    return taskManager.getTasks().stream().filter(GuiTask::isRunning).toList();
+  }
+
+  private String buildStatusText() {
+
+    List<GuiTask> runningTasks = getRunningTasks();
+    if (runningTasks.size() > 1) {
+      return runningTasks.size() + " tasks running...";
+    } else if (runningTasks.size() == 1) {
+      return runningTasks.getFirst().displayTextProperty().get();
+    }
+    List<GuiTask> tasks = taskManager.getTasks();
+    if (!tasks.isEmpty()) {
+      return buildFinishedSummary(tasks);
+    }
+    return "IDEasy is ready.";
+  }
+
+  private double buildStatusProgress() {
+
+    List<GuiTask> runningTasks = getRunningTasks();
+    return (runningTasks.size() == 1) ? runningTasks.getFirst().progressProperty().get() : 0.0;
   }
 
   private void initLanguageComboBox() {
@@ -254,28 +310,23 @@ private void openIDE(String inIde) {
 
   private Task<Void> runIdeCommandTask(String inIde) {
 
-    try (ProgressBarTask task = (ProgressBarTask) guiStateManager.getCurrentContext()
-        .newProgressBarIndeterminate("Starting " + inIde)) {
-      Task<Void> downloadTask = new Task<>() {
-        @Override
-        protected Void call() {
-          guiStateManager
-              .getCurrentContext()
-              .getCommandletManager()
-              .getCommandlet(inIde)
-              .run();
-          return null;
-        }
-      };
-
-      downloadTask.setOnFailed(_ -> Platform.runLater(() -> {
-        task.close();
-        IdeDialog errorDialog = new IdeDialog(AlertType.ERROR, "Error occurred while launching " + inIde);
-        errorDialog.showAndWait();
-      }));
-      downloadTask.setOnSucceeded(_ -> Platform.runLater(task::close));
-      return downloadTask;
-    }
+    Task<Void> downloadTask = new Task<>() {
+      @Override
+      protected Void call() {
+        // Each execution gets its own context so that concurrently started commands keep independent step stacks.
+        IdeGuiContext runContext = guiStateManager.newRunContext();
+        // Wrapping the commandlet in a root step gives the user a single task that reports on all steps the commandlet creates below it.
+        runContext.newStep("Starting " + inIde).run(() ->
+            runContext.getCommandletManager().getCommandlet(inIde).run());
+        return null;
+      }
+    };
+
+    downloadTask.setOnFailed(_ -> Platform.runLater(() -> {
+      IdeDialog errorDialog = new IdeDialog(AlertType.ERROR, "Error occurred while launching " + inIde);
+      errorDialog.showAndWait();
+    }));
+    return downloadTask;
   }
 
   private void updateContext(String selectedProjectName, String selectedWorkspaceName) {
@@ -288,48 +339,13 @@ private void updateContext(String selectedProjectName, String selectedWorkspaceN
     }
   }
 
-  private void updateStatusLabel(List<ProgressBarTask> taskList) {
-
-    Platform.runLater(() -> {
-
-      if (taskList.size() > 1) {
-        statusLabel.setOnMouseClicked(e -> TaskOverviewWindow.getInstance(taskManager).showRelativeToReferenceNode(statusLabel));
-
-        statusProgressBar.setVisible(false);
-        statusProgressBar.setPrefWidth(0);
-        statusLabel.setText(taskList.size() + " tasks running...");
-
-        statusLabel.setUnderline(true);
-        statusLabel.setStyle(
-            "-fx-text-fill: blue;"
-                + "-fx-cursor: hand"
-        );
-      } else if (taskList.size() == 1) {
-        statusLabel.setOnMouseClicked(null);
-
-        ProgressBarTask task = taskList.getFirst();
-        statusLabel.setText(String.format(
-            ProgressBarTask.TASK_DESCRIPTION_STRING_FORMAT,
-            task.getTitle(),
-            task.getCurrentProgress(),
-            task.getMaxSize(),
-            task.getUnitName())
-        );
-        statusLabel.setUnderline(false);
-        statusLabel.setStyle("");
-
-        statusProgressBar.setVisible(true);
-        statusProgressBar.setPrefWidth(PROGRESSBAR_VISIBLE_WIDTH);
-        statusProgressBar.setProgress((double) (task.getCurrentProgress()) / task.getMaxSize());
-      } else {
-        statusLabel.setOnMouseClicked(null);
-        statusLabel.setText("IDEasy is ready.");
-        statusProgressBar.setVisible(false);
-        statusProgressBar.setPrefWidth(0);
-
-        statusLabel.setUnderline(false);
-        statusLabel.setStyle("");
-      }
-    });
+  private String buildFinishedSummary(List<GuiTask> taskList) {
+
+    long failed = taskList.stream().filter(task -> task.getState() == TaskState.FAILED).count();
+    if (failed > 0) {
+      return String.format("%d of %d tasks failed", failed, taskList.size());
+    }
+    return String.format("%d tasks finished", taskList.size());
   }
+
 }