fix(bar): update widget selection index after deletion

meson.build

@@ -1164,6 +1164,7 @@ if build_tests
     'jxl_decoder',
     'keyboard_layout_label',
     'kde_color_scheme',
+    'lane_selection_token',
     'log',
     'location_service',
     'math_provider',

src/shell/settings/bar_widget_editor.cpp

@@ -22,6 +22,7 @@
 
 #include <algorithm>
 #include <cctype>
+#include <charconv>
 #include <cmath>
 #include <cstddef>
 #include <cstdint>
@@ -1791,14 +1792,15 @@ namespace settings {
                     .paddingV = Style::spaceXs * ctx.scale,
                     .paddingH = Style::spaceSm * ctx.scale,
                     .radius = Style::scaledRadiusSm(ctx.scale),
-                    .onClick = [setOverrides = ctx.setOverrides, sourceItems, sourcePath, targetItems, targetPath,
-                                widgetName]() mutable {
+                    .onClick = [&selectedLaneWidgets = ctx.selectedLaneWidgets, setOverrides = ctx.setOverrides,
+                                sourceItems, sourcePath, targetItems, targetPath, widgetName]() mutable {
                       auto it = std::ranges::find(sourceItems, widgetName);
                       if (it == sourceItems.end()) {
                         return;
                       }
                       sourceItems.erase(it);
                       targetItems.push_back(widgetName);
+                      selectedLaneWidgets.clear();
                       setOverrides({{sourcePath, sourceItems}, {targetPath, targetItems}});
                     },
                 })
@@ -1968,12 +1970,14 @@ namespace settings {
                       .paddingV = Style::spaceXs * ctx.scale,
                       .paddingH = Style::spaceSm * ctx.scale,
                       .radius = Style::scaledRadiusSm(ctx.scale),
-                      .onClick = [&pendingDeleteWidgetName = ctx.pendingDeleteWidgetName, config = ctx.config,
-                                  widgetName, clearOverride = ctx.clearOverride, setOverrides = ctx.setOverrides,
+                      .onClick = [&pendingDeleteWidgetName = ctx.pendingDeleteWidgetName,
+                                  &selectedLaneWidgets = ctx.selectedLaneWidgets, config = ctx.config, widgetName,
+                                  clearOverride = ctx.clearOverride, setOverrides = ctx.setOverrides,
                                   closeHostedEditor = ctx.closeHostedEditor]() {
                         pendingDeleteWidgetName.clear();
                         auto referenceRemovals = widgetReferenceRemovalOverrides(config, widgetName);
                         if (!referenceRemovals.empty()) {
+                          selectedLaneWidgets.clear();
                           setOverrides(std::move(referenceRemovals));
                         }
                         clearOverride({"widget", widgetName});
@@ -2386,8 +2390,8 @@ namespace settings {
               .paddingV = Style::spaceXs * ctx.scale,
               .paddingH = Style::spaceSm * ctx.scale,
               .radius = Style::scaledRadiusSm(ctx.scale),
-              .onClick = [setOverrides = ctx.setOverrides, groupId, groupPath, laneListPath, config = &ctx.config,
-                          closeHostedEditor = ctx.closeHostedEditor]() {
+              .onClick = [&selectedLaneWidgets = ctx.selectedLaneWidgets, setOverrides = ctx.setOverrides, groupId,
+                          groupPath, laneListPath, config = &ctx.config, closeHostedEditor = ctx.closeHostedEditor]() {
                 std::vector<BarCapsuleGroupStyle> currentGroups = capsuleGroupsForLanePath(*config, laneListPath);
                 const BarCapsuleGroupStyle* g = findCapsuleGroupStyle(currentGroups, groupId);
                 if (g == nullptr) {
@@ -2422,6 +2426,7 @@ namespace settings {
                   }
                 }
                 batch.emplace_back(groupPath, remaining);
+                selectedLaneWidgets.clear();
                 setOverrides(std::move(batch));
                 if (closeHostedEditor) {
                   closeHostedEditor();
@@ -2449,17 +2454,16 @@ namespace settings {
       std::string laneKey;
       std::vector<std::size_t> indices;
       for (const auto& token : selection) {
-        const auto hash = token.find('#');
-        if (hash == std::string::npos) {
+        const auto parsed = parseLaneSelectionToken(token);
+        if (!parsed.has_value()) {
           return plan;
         }
-        const std::string key = token.substr(0, hash);
         if (laneKey.empty()) {
-          laneKey = key;
-        } else if (laneKey != key) {
+          laneKey = parsed->laneKey;
+        } else if (laneKey != parsed->laneKey) {
           return plan; // selection spans multiple lanes
         }
-        indices.push_back(static_cast<std::size_t>(std::strtoul(token.c_str() + hash + 1, nullptr, 10)));
+        indices.push_back(parsed->index);
       }
       std::ranges::sort(indices);
 
@@ -2581,6 +2585,47 @@ namespace settings {
     return isBarWidgetListPath(path) && path.back() == "start";
   }
 
+  std::string makeLaneSelectionToken(std::string_view laneKey, std::size_t index) {
+    return std::string(laneKey) + "#" + std::to_string(index);
+  }
+
+  std::optional<LaneSelectionToken> parseLaneSelectionToken(std::string_view token) {
+    const auto hash = token.find('#');
+    if (hash == std::string_view::npos || hash == 0 || hash + 1 == token.size()) {
+      return std::nullopt;
+    }
+    const std::string_view digits = token.substr(hash + 1);
+    std::size_t index = 0;
+    const auto parsed = std::from_chars(digits.data(), digits.data() + digits.size(), index);
+    if (parsed.ec != std::errc{} || parsed.ptr != digits.data() + digits.size()) {
+      return std::nullopt;
+    }
+    return LaneSelectionToken{.laneKey = token.substr(0, hash), .index = index};
+  }
+
+  void reindexLaneSelectionAfterRemoval(
+      std::vector<std::string>& selection, std::string_view laneKey, std::size_t removedIndex
+  ) {
+    std::vector<std::string> kept;
+    kept.reserve(selection.size());
+    for (auto& token : selection) {
+      const auto parsed = parseLaneSelectionToken(token);
+      if (!parsed.has_value() || parsed->laneKey != laneKey) {
+        kept.push_back(std::move(token));
+        continue;
+      }
+      if (parsed->index == removedIndex) {
+        continue;
+      }
+      if (parsed->index > removedIndex) {
+        kept.push_back(makeLaneSelectionToken(laneKey, parsed->index - 1));
+      } else {
+        kept.push_back(std::move(token));
+      }
+    }
+    selection.swap(kept);
+  }
+
   void buildWidgetInspectorBody(
       Flex& body, const std::vector<std::string>& laneListPath, const BarWidgetEditorContext& ctx
   ) {
@@ -2644,7 +2689,8 @@ namespace settings {
                         std::size_t itemIndex
                     ) {
       auto dragState = std::make_shared<LaneWidgetDragState>();
-      handle.setOnPress([dragState, cardPtr, zones, config = &ctx.config, laneListPath, setOverride = ctx.setOverride,
+      handle.setOnPress([dragState, cardPtr, zones, config = &ctx.config, laneListPath,
+                         &selectedLaneWidgets = ctx.selectedLaneWidgets, setOverride = ctx.setOverride,
                          setOverrides = ctx.setOverrides, homeZoneIndex,
                          itemIndex](float localX, float localY, bool pressed) {
         const auto clearHighlight = [&]() {
@@ -2680,7 +2726,10 @@ namespace settings {
         if (!dragState->moved) {
           return;
         }
+        // A move or combine renumbers lane positions, so index-keyed selection tokens no longer
+        // address the widgets the user picked.
         if (dragState->combineZoneIndex.has_value() && dragState->combineItemIndex.has_value()) {
+          selectedLaneWidgets.clear();
           createGroupByCombine(
               *config, *zones, homeZoneIndex, itemIndex, *dragState->combineZoneIndex, *dragState->combineItemIndex,
               setOverrides
@@ -2690,6 +2739,7 @@ namespace settings {
         if (!dragState->targetZoneIndex.has_value() || !dragState->targetInsertionIndex.has_value()) {
           return;
         }
+        selectedLaneWidgets.clear();
         performZoneMove(
             *config, laneListPath, *zones, homeZoneIndex, itemIndex, *dragState->targetZoneIndex,
             *dragState->targetInsertionIndex, setOverride, setOverrides
@@ -3030,9 +3080,13 @@ namespace settings {
       }
       // Reset reverts the whole lane: its widget list and the capsule groups it holds.
       if (overridden || (monitorLaneExplicit && hasGuiOverride)) {
-        laneHeader->addChild(ctx.makeResetActionButton(lanePath, [resetBarLane = ctx.resetBarLane, lanePath]() {
-          resetBarLane(lanePath);
-        }));
+        laneHeader->addChild(ctx.makeResetActionButton(
+            lanePath, [&selectedLaneWidgets = ctx.selectedLaneWidgets, resetBarLane = ctx.resetBarLane, lanePath]() {
+              // Lane contents are replaced wholesale; every index-keyed token in it is stale.
+              selectedLaneWidgets.clear();
+              resetBarLane(lanePath);
+            }
+        ));
       }
       lane->addChild(std::move(laneHeader));
 
@@ -3067,8 +3121,10 @@ namespace settings {
                       .minHeight = Style::controlHeightSm * ctx.scale,
                       .padding = Style::spaceXs * ctx.scale,
                       .radius = Style::scaledRadiusSm(ctx.scale),
-                      .onClick = [setOverride = ctx.setOverride, items = laneItems, lanePath, i]() mutable {
+                      .onClick = [&selectedLaneWidgets = ctx.selectedLaneWidgets, setOverride = ctx.setOverride,
+                                  items = laneItems, lanePath, laneKey, i]() mutable {
                         items.erase(items.begin() + static_cast<std::ptrdiff_t>(i));
+                        reindexLaneSelectionAfterRemoval(selectedLaneWidgets, laneKey, i);
                         setOverride(lanePath, items);
                       },
                   })
@@ -3199,7 +3255,8 @@ namespace settings {
                     .minHeight = iconSize,
                     .padding = iconPad,
                     .radius = Style::scaledRadiusSm(ctx.scale),
-                    .onClick = [config = &ctx.config, lanePath, gid, setOverrides = ctx.setOverrides]() {
+                    .onClick = [&selectedLaneWidgets = ctx.selectedLaneWidgets, config = &ctx.config, lanePath, gid,
+                                setOverrides = ctx.setOverrides]() {
                       std::vector<BarCapsuleGroupStyle> groups = capsuleGroupsForLanePath(*config, lanePath);
                       const BarCapsuleGroupStyle* g = findCapsuleGroupStyle(groups, gid);
                       if (g == nullptr) {
@@ -3226,6 +3283,7 @@ namespace settings {
                           remaining.push_back(x);
                         }
                       }
+                      selectedLaneWidgets.clear();
                       setOverrides({{lanePath, laneEntries}, {groupPath, remaining}});
                     },
                 })
@@ -3265,7 +3323,8 @@ namespace settings {
           for (std::size_t m = 0; m < group->members.size(); ++m) {
             std::function<void()> eject;
             if (!inherited) {
-              eject = [config = &ctx.config, lanePath, gid, m, setOverrides = ctx.setOverrides]() {
+              eject = [&selectedLaneWidgets = ctx.selectedLaneWidgets, config = &ctx.config, lanePath, gid, m,
+                       setOverrides = ctx.setOverrides]() {
                 const std::vector<std::string> groupPath = capsuleGroupPathForLanePath(lanePath);
                 if (groupPath.empty()) {
                   return;
@@ -3305,6 +3364,7 @@ namespace settings {
                 }
                 insertAt = std::min(insertAt, laneEntries.size());
                 laneEntries.insert(laneEntries.begin() + static_cast<std::ptrdiff_t>(insertAt), ejected);
+                selectedLaneWidgets.clear();
                 setOverrides({{lanePath, laneEntries}, {groupPath, groups}});
               };
             }
@@ -3322,16 +3382,18 @@ namespace settings {
         }
 
         // Loose widget card.
-        const std::string selectionToken = std::string(laneKey) + "#" + std::to_string(i);
+        const std::string selectionToken = makeLaneSelectionToken(laneKey, i);
         const bool isSelected = std::ranges::contains(ctx.selectedLaneWidgets, selectionToken);
         std::function<void()> removeClose;
         if (!inherited) {
           auto items = laneItems;
           items.erase(items.begin() + static_cast<std::ptrdiff_t>(i));
           const bool removeInstance = isGuiManagedNamedWidgetInstance(ctx, entryName)
               && !widgetHasPlacementAfterLaneEdit(ctx.config, lanePath, items, entryName);
-          removeClose = [setOverride = ctx.setOverride, clearOverride = ctx.clearOverride, items = std::move(items),
-                         lanePath, entryName, removeInstance]() {
+          removeClose = [&selectedLaneWidgets = ctx.selectedLaneWidgets, setOverride = ctx.setOverride,
+                         clearOverride = ctx.clearOverride, items = std::move(items), lanePath, entryName,
+                         removeInstance, laneKey, i]() {
+            reindexLaneSelectionAfterRemoval(selectedLaneWidgets, laneKey, i);
             setOverride(lanePath, items);
             if (removeInstance) {
               clearOverride({"widget", entryName});

src/shell/settings/bar_widget_editor.h

@@ -2,10 +2,12 @@
 
 #include "shell/settings/settings_registry.h"
 
+#include <cstddef>
 #include <functional>
 #include <memory>
 #include <optional>
 #include <string>
+#include <string_view>
 #include <vector>
 
 class Button;
@@ -77,6 +79,21 @@ namespace settings {
   [[nodiscard]] bool isBarWidgetListPath(const std::vector<std::string>& path);
   [[nodiscard]] bool isFirstBarWidgetListPath(const std::vector<std::string>& path);
 
+  // Lane selection tokens address a lane position as "<laneKey>#<index>".
+  struct LaneSelectionToken {
+    std::string_view laneKey;
+    std::size_t index = 0;
+  };
+
+  [[nodiscard]] std::string makeLaneSelectionToken(std::string_view laneKey, std::size_t index);
+  // nullopt unless `token` is well-formed; `laneKey` views into `token`.
+  [[nodiscard]] std::optional<LaneSelectionToken> parseLaneSelectionToken(std::string_view token);
+  // Drops the token addressing `removedIndex` in `laneKey` and shifts that lane's higher indices
+  // down one. Tokens for other lanes are left alone.
+  void reindexLaneSelectionAfterRemoval(
+      std::vector<std::string>& selection, std::string_view laneKey, std::size_t removedIndex
+  );
+
   void addBarWidgetLaneEditor(Flex& section, const SettingEntry& entry, const BarWidgetEditorContext& ctx);
 
   // Builds a BarWidgetEditorContext whose control factories delegate to `factory`. The returned

tests/lane_selection_token_test.cpp

@@ -0,0 +1,73 @@
+#include "shell/settings/bar_widget_editor.h"
+#include "tests/test_check.h"
+
+#include <string>
+#include <vector>
+
+namespace {
+
+  using settings::makeLaneSelectionToken;
+  using settings::parseLaneSelectionToken;
+  using settings::reindexLaneSelectionAfterRemoval;
+
+  void testTokenRoundTrip() {
+    TEST_CHECK(makeLaneSelectionToken("start", 0) == "start#0");
+    TEST_CHECK(makeLaneSelectionToken("end", 12) == "end#12");
+
+    const auto parsed = parseLaneSelectionToken("center#3");
+    TEST_CHECK(parsed.has_value());
+    TEST_CHECK(parsed->laneKey == "center");
+    TEST_CHECK(parsed->index == 3);
+  }
+
+  void testMalformedTokensAreRejected() {
+    // A token that does not parse must not be mistaken for lane position 0.
+    TEST_CHECK(!parseLaneSelectionToken("start").has_value());
+    TEST_CHECK(!parseLaneSelectionToken("").has_value());
+    TEST_CHECK(!parseLaneSelectionToken("#2").has_value());
+    TEST_CHECK(!parseLaneSelectionToken("start#").has_value());
+    TEST_CHECK(!parseLaneSelectionToken("start#x").has_value());
+    TEST_CHECK(!parseLaneSelectionToken("start#2x").has_value());
+    TEST_CHECK(!parseLaneSelectionToken("start#-1").has_value());
+    TEST_CHECK(!parseLaneSelectionToken("start# 2").has_value());
+  }
+
+  void testReindexAfterRemoval() {
+    // Removing lane position 1: it drops out, higher positions shift down, lower ones stay.
+    std::vector<std::string> selection = {"start#0", "start#1", "start#2", "start#4"};
+    reindexLaneSelectionAfterRemoval(selection, "start", 1);
+    TEST_CHECK(selection == std::vector<std::string>({"start#0", "start#1", "start#3"}));
+  }
+
+  void testReindexLeavesOtherLanesAlone() {
+    std::vector<std::string> selection = {"center#2", "start#3", "end#0"};
+    reindexLaneSelectionAfterRemoval(selection, "start", 0);
+    TEST_CHECK(selection == std::vector<std::string>({"center#2", "start#2", "end#0"}));
+  }
+
+  void testReindexKeepsUnrelatedEntries() {
+    // Nothing addresses the edited lane: the selection is untouched.
+    std::vector<std::string> selection = {"end#1", "end#2"};
+    reindexLaneSelectionAfterRemoval(selection, "start", 0);
+    TEST_CHECK(selection == std::vector<std::string>({"end#1", "end#2"}));
+
+    // Unparseable entries cannot address a card, so they are carried through rather than reinterpreted.
+    std::vector<std::string> malformed = {"start", "start#1"};
+    reindexLaneSelectionAfterRemoval(malformed, "start", 1);
+    TEST_CHECK(malformed == std::vector<std::string>({"start"}));
+
+    std::vector<std::string> empty;
+    reindexLaneSelectionAfterRemoval(empty, "start", 0);
+    TEST_CHECK(empty.empty());
+  }
+
+} // namespace
+
+int main() {
+  testTokenRoundTrip();
+  testMalformedTokensAreRejected();
+  testReindexAfterRemoval();
+  testReindexLeavesOtherLanesAlone();
+  testReindexKeepsUnrelatedEntries();
+  return 0;
+}