cleanup

resources/web/dialog/PluginsConfigDialog/index.html

@@ -5,9 +5,7 @@
   <meta name="viewport" content="width=device-width, initial-scale=1">
   <title>Plugin configuration</title>
   <link rel="stylesheet" href="styles.css">
-  <!-- Same three shared sheets every resources/web/dialog/* page links (see PluginsDialog/index.html):
+  <!-- The shared dialog sheets, theme.css last so its host-injected variables win. -->
-       global.css for buttons/scrollbars, common.css for the base reset, theme.css last so its
-       host-injected --orca-* contract wins and themes the .config-* rules ported into styles.css. -->
   <link rel="stylesheet" type="text/css" href="../../include/global.css">
   <link rel="stylesheet" type="text/css" href="../css/common.css">
   <link rel="stylesheet" type="text/css" href="../css/theme.css">
@@ -34,8 +32,7 @@
             autocomplete="off" autocapitalize="off" aria-label="Capability configuration (JSON)"></textarea>
         </div>
         <!-- Custom capability UI. Sandboxed without allow-same-origin, so the plugin's HTML runs in
-             an opaque origin: it cannot touch this page, and the only host surface it gets is the
+             an opaque origin and reaches the host only through the injected window.orca bridge. -->
-             window.orca getConfig/saveConfig bridge injected into srcdoc. -->
         <iframe id="configCustom" class="config-custom" title="Plugin configuration"
           sandbox="allow-scripts" referrerpolicy="no-referrer" hidden></iframe>
         <div id="configFooter" class="config-view-footer" hidden>

resources/web/dialog/PluginsConfigDialog/index.js

@@ -1,10 +1,8 @@
-// The capabilities the active preset uses, as sent by PluginsConfigDialog::send_capabilities.
+// Capability rows the active preset uses, as PluginConfig::capabilities_payload emits them:
-// Each row is {plugin_key, name, type, type_key, has_config_ui} — the shape
+// {plugin_key, name, type, type_key, has_config_ui}.
-// PluginConfig::capabilities_payload emits, shared with the Plugins dialog's Config tab.
 let capabilities = [];
 
-// The selected row's full identity. plugin_key is part of it because this list spans plugins,
+// The selected row's identity; plugin_key is part of it because this list spans plugins.
-// unlike the Plugins dialog's config tab where every row belongs to the one selected plugin.
 let selectedPluginKey = "";
 let selectedCapabilityName = "";
 let selectedCapabilityType = "";
@@ -105,8 +103,7 @@ function RenderCapabilities() {
   empty.hidden = true;
   layout.hidden = false;
 
-  // Keep the selection across a refresh when the capability is still there; otherwise fall back to
+  // Keep the selection across a refresh if the capability is still there, else select the first.
-  // the first one, which is also the initial selection.
   if (!capabilities.some(IsSameCapability)) {
     selectedPluginKey = String(capabilities[0].plugin_key || "");
     selectedCapabilityName = String(capabilities[0].name || "");
@@ -158,26 +155,17 @@ function OnConfigSidebarClick(event) {
   selectedCapabilityName = name;
   selectedCapabilityType = typeKey;
 
-  // Drop the outgoing capability's view immediately: the native reply is asynchronous and its
+  // The native reply is async: clear now so the old config cannot appear under the new selection.
-  // content must never appear under the newly selected capability.
   ClearCapabilityConfigView();
   RequestCapabilityConfig();
   RenderCapabilities();
 }
 
-// ---------------------------------------------------------------------------
+// Config editor: the host's JSON editor, or the capability's own HTML UI in a sandboxed frame.
-// Config editor
+// Both edit the same stored config; the page renders what the native side sends.
-//
-// The right side shows either the host's JSON editor or, when the capability ships one, its own
-// HTML UI in a sandboxed frame. Both edit the same stored config: the page holds no config state
-// of its own, it renders what the native side sends and sends back what the user saves. Ported
-// from PluginsDialog/index.js, whose config tab lives inside a single selected plugin (it reads
-// selectedPluginId from a page-global); this list spans plugins, so every row carries its own
-// plugin_key and the page tracks selectedPluginKey instead.
-// ---------------------------------------------------------------------------
 
-// A reply is only applied when it still matches the selected row. The native side is asynchronous,
+// Replies are async: apply one only if it still matches the selected row (plugin_key included,
-// and unlike the Plugins dialog this list spans plugins, so plugin_key is part of the match.
+// since this list spans plugins), so a stale reply never lands under another capability.
 function IsCurrentCapability(payload) {
   return String(payload?.plugin_key || "") === selectedPluginKey
     && String(payload?.capability_name || "") === selectedCapabilityName
@@ -195,9 +183,8 @@ function RequestCapabilityConfig() {
   });
 }
 
-// Empties both editors, so nothing from the previously selected capability can linger while the
+// Empties both editors and the footer, so nothing from the previous capability lingers while the
-// next one is still in flight. The footer goes with them: until a config has actually loaded there
+// next one is in flight.
-// is nothing to save or restore.
 function ClearCapabilityConfigView() {
   const editor = document.getElementById("configEditor");
   const custom = document.getElementById("configCustom");
@@ -243,7 +230,6 @@ function UpdateConfigMeta(payload) {
     (source === "base" ? "Inherited from global configuration" : "No saved configuration");
   if (save)
     save.disabled = selectedReadOnly;
-  // There is nothing to discard when the preset holds no override of its own.
   if (restore)
     restore.disabled = selectedReadOnly || !selectedHasPresetOverride;
   meta.hidden = false;
@@ -268,12 +254,10 @@ function ApplyCapabilityConfig(payload) {
   const html = String(payload?.custom_html || "");
   UpdateConfigMeta(payload);
 
-  // Restore is host chrome and applies to either editor; Save and the validation message belong to
+  // Restore applies to either editor; Save and validation belong to the JSON editor only.
-  // the JSON editor, since a custom UI saves through its own controls via the bridge.
   ShowConfigFooter(!html);
 
   if (html) {
-    // A capability with its own UI: hand it the config through the bridge, never the raw file.
     if (custom) {
       custom.hidden = false;
       custom.srcdoc = BuildCustomConfigDocument(html, config);
@@ -283,8 +267,7 @@ function ApplyCapabilityConfig(payload) {
     return;
   }
 
-  // Default editor. The native side already reported why a custom UI is unavailable (if it was
+  // Default editor: any reason a custom UI is unavailable already arrived in payload.error.
-  // meant to have one) in payload.error, and we fall back to editing the same config here.
   if (custom) {
     custom.hidden = true;
     custom.removeAttribute("srcdoc");
@@ -296,8 +279,7 @@ function ApplyCapabilityConfig(payload) {
   SetConfigValidation("");
 }
 
-// Reveals the footer for the loaded capability. `withEditorControls` is false for a custom UI,
+// withEditorControls is false for a custom UI, which saves through its own controls.
-// leaving Restore on its own.
 function ShowConfigFooter(withEditorControls) {
   const footer = document.getElementById("configFooter");
   const save = document.getElementById("configSaveBtn");
@@ -318,8 +300,7 @@ function SetConfigValidation(message) {
     node.textContent = message;
     node.classList.toggle("invalid", message !== "");
   }
-  // Invalid JSON can never be saved: the button is the only way to persist, and it is disabled
+  // Invalid JSON is never saved: Save is the only way to persist. The native side re-validates.
-  // while the text does not parse. The native side re-validates regardless.
   if (save)
     save.disabled = selectedReadOnly || message !== "";
 }
@@ -359,10 +340,8 @@ function SaveCapabilityConfig() {
   });
 }
 
-// Drops the preset's override, which is what "default" means here: the capability falls back to the
+// "Restore defaults" here drops the preset's override, so the capability falls back to the global
-// global configuration, the same as a preset that was never overridden. The native side confirms
+// configuration. The native side confirms, then re-sends the config that is now effective.
-// before discarding it and then re-sends the capability's config, so the editors reload from what is
-// now effective rather than from what was typed.
 function RestoreCapabilityConfig() {
   if (!selectedPluginKey || !selectedCapabilityName || !selectedHasPresetOverride)
     return;
@@ -387,8 +366,7 @@ function ApplyCapabilityConfigSaved(payload) {
   if (payload?.ok !== true)
     return;
 
-  // Reload from what was actually persisted, so both editors show the stored state rather than
+  // Reload from what was persisted, not from what was typed.
-  // whatever was typed.
   const config = payload && Object.prototype.hasOwnProperty.call(payload, "config") ? payload.config : {};
   const custom = document.getElementById("configCustom");
   const text = document.getElementById("configText");
@@ -401,13 +379,11 @@ function ApplyCapabilityConfigSaved(payload) {
   SetConfigValidation("");
 }
 
-// The whole host surface a custom config UI gets: read the config it was opened with, save a new
+// The whole host surface a custom config UI gets: read the config, save one, be told when a save
-// one, and be told when a save lands. Everything else about the dialog stays out of reach — the
+// lands. The frame is sandboxed into an opaque origin, so this bridge is its only channel.
-// frame is sandboxed into an opaque origin, so this bridge is its only way to talk to the host.
 function BuildCustomConfigDocument(html, config) {
-  // The config is inlined into a <script>, so a stored string containing "</script>" would
+  // Inlined into a <script>: a stored "</script>" would close the tag early, so escape "<" — the
-  // otherwise close the tag early and inject the rest as markup. Escaping "<" keeps the literal
+  // literal stays valid JSON.
-  // valid JSON while making that impossible.
   const seed = JSON.stringify(config).replace(/</g, "\\u003c");
   const bridge = `<script>
 (function () {
@@ -446,8 +422,6 @@ function OnCustomConfigMessage(event) {
   if (!selectedPluginKey || !selectedCapabilityName)
     return;
 
-  // The custom UI persists through the same native command as the JSON editor, so there is one
-  // stored config and one code path that writes it.
   SendMessage("save_capability_config", {
     plugin_key: selectedPluginKey,
     capability_name: selectedCapabilityName,
@@ -473,9 +447,9 @@ document.addEventListener("DOMContentLoaded", () => {
   if (text)
     text.addEventListener("input", ValidateConfigText);
 
-  // The custom capability UI is sandboxed into an opaque origin, so it reaches us only through
+  // The custom UI is sandboxed into an opaque origin, so postMessage is its only channel.
-  // postMessage. OnCustomConfigMessage matches on the frame's own contentWindow rather than the
+  // OnCustomConfigMessage matches on the frame's contentWindow, not the origin ("null" when
-  // origin (which is "null" for a sandboxed frame) and ignores anything else on the channel.
+  // sandboxed), and ignores anything else.
   window.addEventListener("message", OnCustomConfigMessage);
 
   SendMessage("request_capabilities");

resources/web/dialog/PluginsDialog/index.html

@@ -153,15 +153,12 @@
                     autocomplete="off" autocapitalize="off" aria-label="Capability configuration (JSON)"></textarea>
                 </div>
                 <!-- Custom capability UI. Sandboxed without allow-same-origin, so the plugin's HTML
-                     runs in an opaque origin: it cannot touch this page, and the only host surface
+                     runs in an opaque origin and reaches the host only through the injected
-                     it gets is the window.orca getConfig/saveConfig bridge injected into srcdoc. -->
+                     window.orca bridge. -->
                 <iframe id="configCustom" class="config-custom" title="Plugin configuration"
                   sandbox="allow-scripts" referrerpolicy="no-referrer" hidden></iframe>
-                <!-- Host chrome for both editors, not just the JSON one: a capability with a custom
+                <!-- Host chrome for both editors: a custom UI needs Restore too, and keeping it here
-                     UI needs Restore just as much, and keeping it here leaves it out of the plugin's
+                     leaves it off the JS bridge. Save and validation are JSON-editor only. -->
-                     HTML and off the JS bridge. Save and the validation message belong to the JSON
-                     editor alone (a custom UI saves through its own controls), so they are hidden
-                     when a custom UI is showing. -->
                 <div id="configFooter" class="config-view-footer" hidden>
                   <span id="configValidation" class="config-validation" role="status" aria-live="polite"></span>
                   <div class="config-actions">

resources/web/dialog/PluginsDialog/index.js

@@ -12,22 +12,18 @@ const pluginInstallActions = {
 
 let expandedPluginIds = new Set();
 
-// why: transient per-search override on top of expandedPluginIds. A search
+// why: transient per-search override (id -> bool) on top of expandedPluginIds. A search auto-expands
-//      auto-expands rows whose capabilities match, display-only. This lets a
+//      matching rows, so a triangle click during search must not touch the base expand state.
-//      triangle click during search collapse/reopen such a row without touching
-//      the base (id -> bool).
 let searchExpandOverride = new Map();
 let selectedPluginId = "";
 let contextPluginId = "";
 let activeDetailTab = "plugin-info";
 let selectedInstallAction = "explore";
-// Config tab: the capability whose config is currently shown, scoped to selectedPluginId. Name and
+// Config tab: the capability whose config is shown. Name and type together address it natively.
-// type together address the capability on the native side. Cleared whenever the plugin changes.
 let selectedCapabilityName = "";
 let selectedCapabilityType = "";
-// The plugin the capability selection above belongs to. Capability names are only unique within a
+// The plugin that selection belongs to. Capability names are only unique within a plugin, so
-// plugin, so two plugins can each expose e.g. a "main" script capability; without remembering the
+// without the owner, selecting another plugin could keep the selection and show the wrong config.
-// owner, selecting the second plugin would keep the selection and show the first plugin's config.
 let configPluginId = "";
 
 let pluginList = null;
@@ -73,15 +69,14 @@ function OnInit() {
   document.getElementById("configRestoreBtn")?.addEventListener("click", RestoreCapabilityConfig);
 
   const configText = document.getElementById("configText");
-  // why: common.js installs a document-level onkeydown that cancels the default action of every key
+  // why: common.js cancels every key at the document level (returnValue=false) to block webview
-  //      (returnValue=false) to block webview shortcuts; on the way up it also swallows typing. Stop
+  //      shortcuts, which also swallows typing; don't bubble to it, so the textarea stays editable.
-  //      the editor's keydowns from bubbling to it so the textarea stays editable, leaving the global
+  //      Same treatment as the search field (see plugin-search.js).
-  //      guard intact. Same treatment as the search field (see plugin-search.js).
   configText?.addEventListener("keydown", (event) => event.stopPropagation());
   configText?.addEventListener("input", ValidateConfigText);
-  // The custom capability UI is sandboxed into an opaque origin, so it reaches us only through
+  // The custom UI is sandboxed into an opaque origin, so postMessage is its only channel.
-  // postMessage. Match on the frame's own contentWindow rather than the origin (which is "null"
+  // OnCustomConfigMessage matches on the frame's contentWindow, not the origin ("null" when
-  // for a sandboxed frame) and ignore anything else on the channel.
+  // sandboxed), and ignores anything else.
   window.addEventListener("message", OnCustomConfigMessage);
 
   document.addEventListener("click", (event) => {
@@ -258,8 +253,7 @@ function HandleStudio(value) {
   }
 }
 
-// Renders the latest plugin/capability operation result in the footer status bar. The result
+// Footer status bar for the latest operation result; the native side already localizes the text.
-// persists until the next operation replaces it; the native side already localizes the text.
 function ShowStatusMessage(message, level) {
   const bar = document.getElementById("statusBar");
   const text = document.getElementById("statusText");
@@ -333,7 +327,6 @@ function SyncPluginListHeaderGutter() {
 }
 
 // why: paint matched-character ranges as <mark> without an innerHTML build
-// note: if no ranges -> return the plain text node
 function ApplyHighlight(container, text, ranges) {
   if (!ranges || !ranges.length) {
     container.appendChild(document.createTextNode(text));
@@ -369,7 +362,7 @@ function RenderPlugins() {
   }
 
   // why: stable filter over the existing C++ sort order - no scoring, no reorder. The empty query
-  //      short-circuits (searching=false), leaving every existing render path untouched.
+  //      short-circuits, leaving every existing render path untouched.
   const searching = typeof PluginSearchActive === "function" && PluginSearchActive();
   let shown = 0;
 
@@ -381,10 +374,8 @@ function RenderPlugins() {
       continue;
     shown++;
 
-    // why: transient override wins. Otherwise while searching start collapsed and auto-expand only
+    // why: the transient override wins; while searching, auto-expand only capability matches. The
-    //      capability matches (the persistent expand state is ignored so unrelated caps don't clutter
+    //      base is never written while searching, so clearing it restores exactly what the user had.
-    //      results); when not searching use the persistent state. The base is never written while
-    //      searching, so clearing the search restores exactly what the user had.
     const open = searchExpandOverride.has(pluginKey)
       ? searchExpandOverride.get(pluginKey)
       : (searching ? match.hasCapMatch : expandedPluginIds.has(pluginKey));
@@ -455,7 +446,6 @@ function GetLatestVersion(plugin) {
   return String(plugin?.latest_version || plugin?.version || "");
 }
 
-// Primary version shown in the row: the installed version once installed, otherwise the latest available.
 function GetDisplayVersion(plugin) {
   const installed = GetInstalledVersion(plugin);
   if (IsPluginInstalled(plugin) && installed)
@@ -570,7 +560,6 @@ function LabelCell(plugin, isExpanded = false, capabilityCount = 0, nameRanges =
   const labelCell = document.createElement("span");
   labelCell.className = "label-cell";
 
-  // Add hyperlink
   const hasCloudLink = plugin.source === "mine" || plugin.source === "subscribed";
   const pluginLabelText = plugin.label || plugin.name || plugin.plugin_id || "";
   const canExpand = capabilityCount > 0;
@@ -816,19 +805,12 @@ function RenderDetails() {
   RenderConfig(plugin);
 }
 
-// ---------------------------------------------------------------------------
+// Config tab: the sidebar lists the selected plugin's configurable capabilities; the view shows the
-// Config tab
+// host's JSON editor or, when the capability ships one, its own HTML UI in a sandboxed frame. Both
-//
+// edit the same stored config; the page renders what the native side sends.
-// The sidebar lists the selected plugin's configurable capabilities; the right side shows either
-// the host's JSON editor or, when the capability ships one, its own HTML UI in a sandboxed frame.
-// Both edit the same stored config: the page holds no config state of its own, it renders what the
-// native side sends and sends back what the user saves.
-// ---------------------------------------------------------------------------
 
-// The config sidebar's rows, built natively by PluginConfig::capabilities_payload and shared with
+// Rows built natively by PluginConfig::capabilities_payload. Only loaded, addressable capabilities
-// PluginsConfigDialog. Only loaded, addressable capabilities appear: the descriptor-only rows the
+// appear: the descriptor-only rows of an inactive plugin carry no name, so nothing to address.
-// list tab shows for a plugin that is not activated carry no capability name, so there is nothing to
-// configure and nothing to address on the native side.
 function GetConfigurableCapabilities(plugin) {
   return Array.isArray(plugin?.config_capabilities) ? plugin.config_capabilities : [];
 }
@@ -843,8 +825,7 @@ function RenderConfig(plugin) {
   const capabilities = plugin ? GetConfigurableCapabilities(plugin) : [];
   const pluginKey = String(plugin?.plugin_key || "");
 
-  // A different plugin than the one the current selection belongs to: drop the selection outright
+  // A different plugin: drop the selection rather than trusting the name to mean the same here.
-  // rather than trusting the name to mean the same thing here.
   if (pluginKey !== configPluginId) {
     configPluginId = pluginKey;
     selectedCapabilityName = "";
@@ -853,8 +834,8 @@ function RenderConfig(plugin) {
   }
 
   if (!plugin || capabilities.length === 0) {
-    // Capabilities are only materialized once the plugin is activated, so an inactive plugin has
+    // Capabilities only exist once the plugin is activated: say that, rather than claiming it has
-    // nothing to configure yet — say that, rather than claiming it has no capabilities.
+    // none to configure.
     empty.textContent = !plugin
       ? "Select a plugin to configure its capabilities"
       : (IsPluginLoading(plugin)
@@ -874,8 +855,7 @@ function RenderConfig(plugin) {
   empty.hidden = true;
   layout.hidden = false;
 
-  // Keep the selection across a refresh when the capability is still there; otherwise fall back to
+  // Keep the selection across a refresh if the capability is still there, else select the first.
-  // the first one, which is also the initial selection for a newly selected plugin.
   const stillPresent = capabilities.some((capability) =>
     capability.name === selectedCapabilityName && String(capability.type_key || "") === selectedCapabilityType);
   if (!stillPresent) {
@@ -927,8 +907,7 @@ function OnConfigSidebarClick(event) {
   selectedCapabilityName = name;
   selectedCapabilityType = typeKey;
 
-  // Drop the outgoing capability's view immediately: the native reply is asynchronous and its
+  // The native reply is async: clear now so the old config cannot appear under the new selection.
-  // content must never appear under the newly selected capability.
   ClearCapabilityConfigView();
   RequestCapabilityConfig();
   RenderConfig(pluginsById.get(selectedPluginId));
@@ -945,9 +924,8 @@ function RequestCapabilityConfig() {
   });
 }
 
-// Empties both editors, so nothing from the previously selected capability can linger while the
+// Empties both editors and the footer, so nothing from the previous capability lingers while the
-// next one is still in flight. The footer goes with them: until a config has actually loaded there
+// next one is in flight.
-// is nothing to save or restore.
 function ClearCapabilityConfigView() {
   const editor = document.getElementById("configEditor");
   const custom = document.getElementById("configCustom");
@@ -972,8 +950,8 @@ function ClearCapabilityConfigView() {
   SetConfigValidation("");
 }
 
-// True when a native reply still matches what the user has selected. A reply for a capability the
+// Replies are async: one for a capability the user has navigated away from is dropped, never
-// user has already navigated away from is dropped rather than rendered into the current view.
+// rendered into the current view.
 function IsCurrentCapability(payload) {
   return String(payload?.plugin_key || "") === selectedPluginId &&
     String(payload?.capability_name || "") === selectedCapabilityName;
@@ -997,12 +975,10 @@ function ApplyCapabilityConfig(payload) {
   const config = (payload && typeof payload.config === "object" && payload.config !== null) ? payload.config : {};
   const html = String(payload?.custom_html || "");
 
-  // Restore is host chrome and applies to either editor; Save and the validation message belong to
+  // Restore applies to either editor; Save and validation belong to the JSON editor only.
-  // the JSON editor, since a custom UI saves through its own controls via the bridge.
   ShowConfigFooter(!html);
 
   if (html) {
-    // A capability with its own UI: hand it the config through the bridge, never the raw file.
     if (custom) {
       custom.hidden = false;
       custom.srcdoc = BuildCustomConfigDocument(html, config);
@@ -1012,8 +988,7 @@ function ApplyCapabilityConfig(payload) {
     return;
   }
 
-  // Default editor. The native side already reported why a custom UI is unavailable (if it was
+  // Default editor: any reason a custom UI is unavailable already arrived in payload.error.
-  // meant to have one) in payload.error, and we fall back to editing the same config here.
   if (custom) {
     custom.hidden = true;
     custom.removeAttribute("srcdoc");
@@ -1025,8 +1000,7 @@ function ApplyCapabilityConfig(payload) {
   SetConfigValidation("");
 }
 
-// Reveals the footer for the loaded capability. `withEditorControls` is false for a custom UI,
+// withEditorControls is false for a custom UI, which saves through its own controls.
-// leaving Restore on its own.
 function ShowConfigFooter(withEditorControls) {
   const footer = document.getElementById("configFooter");
   const save = document.getElementById("configSaveBtn");
@@ -1047,8 +1021,7 @@ function SetConfigValidation(message) {
     node.textContent = message;
     node.classList.toggle("invalid", message !== "");
   }
-  // Invalid JSON can never be saved: the button is the only way to persist, and it is disabled
+  // Invalid JSON is never saved: Save is the only way to persist. The native side re-validates.
-  // while the text does not parse. The native side re-validates regardless.
   if (save)
     save.disabled = message !== "";
 }
@@ -1084,10 +1057,8 @@ function SaveCapabilityConfig() {
   });
 }
 
-// Asks the native side to write the capability's default config over whatever is stored. The
+// Writes the capability's own get_default_config() over whatever is stored — the host does not know
-// defaults come from the capability's get_default_config(), never from this page — the host does not
+// what a plugin considers default. The native side confirms first, then replies as if it were a save.
-// know what a given plugin considers default. The native side confirms before discarding anything,
-// and replies with the same "saved" payload, so both editors reload from what was persisted.
 function RestoreCapabilityConfig() {
   if (!selectedPluginId || !selectedCapabilityName)
     return;
@@ -1112,8 +1083,7 @@ function ApplyCapabilityConfigSaved(payload) {
   if (payload?.ok !== true)
     return;
 
-  // Reload from what was actually persisted, so both editors show the stored state rather than
+  // Reload from what was persisted, not from what was typed.
-  // whatever was typed.
   const config = (payload && typeof payload.config === "object" && payload.config !== null) ? payload.config : {};
   const custom = document.getElementById("configCustom");
   const text = document.getElementById("configText");
@@ -1126,13 +1096,11 @@ function ApplyCapabilityConfigSaved(payload) {
   SetConfigValidation("");
 }
 
-// The whole host surface a custom config UI gets: read the config it was opened with, save a new
+// The whole host surface a custom config UI gets: read the config, save one, be told when a save
-// one, and be told when a save lands. Everything else about the dialog stays out of reach — the
+// lands. The frame is sandboxed into an opaque origin, so this bridge is its only channel.
-// frame is sandboxed into an opaque origin, so this bridge is its only way to talk to the host.
 function BuildCustomConfigDocument(html, config) {
-  // The config is inlined into a <script>, so a stored string containing "</script>" would
+  // Inlined into a <script>: a stored "</script>" would close the tag early, so escape "<" — the
-  // otherwise close the tag early and inject the rest as markup. Escaping "<" keeps the literal
+  // literal stays valid JSON.
-  // valid JSON while making that impossible.
   const seed = JSON.stringify(config).replace(/</g, "\\u003c");
   const bridge = `<script>
 (function () {
@@ -1171,8 +1139,6 @@ function OnCustomConfigMessage(event) {
   if (!selectedPluginId || !selectedCapabilityName)
     return;
 
-  // The custom UI persists through the same native command as the JSON editor, so there is one
-  // stored config and one code path that writes it.
   SendMessage("save_capability_config", {
     plugin_key: selectedPluginId,
     capability_name: selectedCapabilityName,
@@ -1203,8 +1169,8 @@ function RenderDescription(plugin) {
 
   node.replaceChildren();
 
-  // Descriptions come only from the plugin's Python header (local/installed plugins). A cloud plugin
+  // Descriptions come from the plugin's Python header; a cloud plugin that is not installed yet has
-  // that is not installed yet has no header, so show a link to view it on OrcaCloud instead.
+  // no header, so link to OrcaCloud instead.
   const description = String(plugin?.description || "").trim();
   if (description && description !== "No description.") {
     node.textContent = description;
@@ -1275,8 +1241,8 @@ function SetText(id, text) {
 function ApplyDetailUpdateBadge(node, plugin) {
   node.className = "version-update-badge";
 
-  // The "update_available" state is represented by the actionable Update button next to the
+  // "update_available" is already the actionable Update button, so the badge only warns about
-  // version, so the detail panel only shows the passive badge for the unauthorized warning.
+  // unauthorized.
   const updateStatus = plugin ? GetUpdateStatus(plugin) : "normal";
   if (updateStatus === "unauthorized") {
     node.hidden = false;
@@ -1376,9 +1342,8 @@ function OnPluginListClick(event) {
 
     const pluginKey = String(block.dataset.pluginKey || "");
     selectedPluginId = pluginKey;
-    // why: during a search the triangle writes to the transient override (read from the on-screen open
+    // why: during a search the triangle writes to the transient override (read from the on-screen
-    //      state), so an auto-expanded row collapses without touching the saved layout. With no search
+    //      open state), so an auto-expanded row collapses without touching the saved layout.
-    //      active, toggle the persistent base exactly as before.
     if (typeof PluginSearchActive === "function" && PluginSearchActive())
       searchExpandOverride.set(pluginKey, !block.classList.contains("expanded"));
     else if (expandedPluginIds.has(pluginKey))

src/libslic3r/PrintConfig.cpp

@@ -828,9 +828,8 @@ void PrintConfigDef::init_common_params()
     def->tooltip = L("Select the network agent implementation for printer communication.");
     def->mode = comAdvanced;
     def->cli = ConfigOptionDef::nocli;
-    // Plugin-backed like the pickers, but edited via a dedicated Choice widget rather than a
+    // Plugin-backed (see ConfigOptionDef::is_plugin_backed), but edited through the Choice widget above
-    // plugin_picker field. plugin_type marks it plugin-backed and names its capability type, so its
+    // rather than a plugin_picker field.
-    // "plugins" manifest reference is derived generically (see ConfigOptionDef::is_plugin_backed).
     def->plugin_type = "printer-connection";
     def->set_default_value(new ConfigOptionString(""));
 

src/slic3r/GUI/Field.cpp

@@ -1996,8 +1996,6 @@ void Choice::msw_rescale()
 
 void PluginField::BUILD()
 {
-    // Wrap the dynamic rows in a single container panel so the field is a proper window-field
-    // (getWindow() != null). The panel owns m_main_sizer; all row controls are children of it.
     auto* panel = new wxPanel(m_parent, wxID_ANY);
     wxGetApp().UpdateDarkUI(panel);
     window = panel;
@@ -2005,7 +2003,6 @@ void PluginField::BUILD()
     m_main_sizer = new wxBoxSizer(wxVERTICAL);
     panel->SetSizer(m_main_sizer);
 
-    // Initialize with default values or empty
     if (m_opt.type == coStrings) {
         const ConfigOptionStrings* vec = m_opt.get_default_value<ConfigOptionStrings>();
         if (vec != nullptr && !vec->values.empty()) {
@@ -2096,13 +2093,11 @@ void PluginField::add_plugin_row(const wxString& value, bool is_last)
     const auto button_size = wxSize(def_width_thinner() * m_em_unit, -1);
     auto row_sizer = new wxBoxSizer(wxHORIZONTAL);
 
-    // Select button with search icon
     ScalableButton* select_btn = new ScalableButton(window, wxID_ANY, "search", wxEmptyString,
         button_size, wxDefaultPosition, wxBU_EXACTFIT | wxNO_BORDER, true, 16);
     wxGetApp().UpdateDarkUI(select_btn);
     select_btn->SetToolTip(_L("Select plugin"));
 
-    // Display text control
     wxTextCtrl* display = new wxTextCtrl(window, wxID_ANY, value,
         wxDefaultPosition, wxSize(def_width_wider() * m_em_unit, wxDefaultCoord),
         wxTE_READONLY);
@@ -2118,7 +2113,6 @@ void PluginField::add_plugin_row(const wxString& value, bool is_last)
         remove_btn->SetToolTip(_L("Remove plugin"));
     }
 
-    // Add button (only on last row)
     ScalableButton* add_btn = nullptr;
     if (is_last && !m_opt.readonly) {
         add_btn = new ScalableButton(window, wxID_ANY, "param_add", wxEmptyString,
@@ -2239,7 +2233,6 @@ void PluginField::set_value(const boost::any& value, bool change_event)
 {
     m_disable_change_event = !change_event;
 
-    // Handle different input types
     if (value.empty()) {
         m_values.clear();
     } else if (value.type() == typeid(std::vector<std::string>)) {
@@ -2312,9 +2305,7 @@ void PluginField::msw_rescale()
 
 namespace {
 
-// The stored text as a document, or an empty array when it is absent or unparseable. Used to compare
+// The stored text as a document, or an empty array when it is absent or unparseable.
-// two versions of the value semantically: re-serializing an unchanged document can reorder keys or
-// drop whitespace, and that alone must not be allowed to mark the preset dirty.
 nlohmann::json plugin_overrides_as_json(const std::string& text)
 {
     if (text.empty())
@@ -2326,12 +2317,8 @@ nlohmann::json plugin_overrides_as_json(const std::string& text)
 
 void PluginConfigField::BUILD()
 {
-    // The button *is* the field's window (the ColourPicker idiom). Wrapping it in a panel would make
-    // this a container that OG_CustomCtrl sizes but never lays out, leaving the button unsized — and
-    // a zero-height row collapses its whole option group out of the page.
     m_button = new ::Button(m_parent, _L("Configure"));
-    // ButtonType::Parameter is the style for a button sitting next to parameter boxes: it is what
+    // ButtonType::Parameter gives the button the same height as the parameter fields above it.
-    // gives the button the same height as the fields above it, which a bare wxButton does not.
     m_button->SetStyle(ButtonStyle::Regular, ButtonType::Parameter);
 
     wxSize size(def_width_wider() * m_em_unit, m_button->GetMinSize().GetHeight());
@@ -2360,8 +2347,6 @@ void PluginConfigField::update_button_label()
     const nlohmann::json entries = plugin_overrides_as_json(m_json);
     const size_t         count   = entries.is_array() ? entries.size() : 0;
 
-    // The count is the whole state this row can show: which capabilities they belong to, and what
-    // they hold, is the dialog's job.
     m_button->SetLabel(count == 0 ? _L("Configure")
                                   : wxString::Format(_L("Configure (%d)"), int(count)));
 }
@@ -2379,16 +2364,14 @@ void PluginConfigField::open_dialog()
         edited = dlg.overrides_json();
     }
 
-    // Only a semantic change is a change: reopening the dialog and closing it must not dirty the
+    // Compare semantically: a round-trip through the serializer can reorder keys or drop whitespace,
-    // preset just because the document round-tripped through the serializer.
+    // and that alone must not dirty the preset.
     if (plugin_overrides_as_json(m_json) == plugin_overrides_as_json(edited))
         return;
 
     m_json  = edited;
     m_value = m_json;
     update_button_label();
-    // The one call that makes this behave like every other setting: it drives change_opt_value into
-    // the preset config, Tab::update_dirty(), and the revert arrow.
     on_change_field();
 }
 

src/slic3r/GUI/Field.hpp

@@ -32,8 +32,7 @@
 #define wxMSW false
 #endif
 
-// Orca's styled button (Widgets/Button.hpp), used by PluginConfigField. Declared at global scope,
+// Orca's styled button (Widgets/Button.hpp), used by PluginConfigField. It lives at global scope.
-// which is where the widget lives.
 class Button;
 
 namespace Slic3r { namespace GUI {
@@ -487,9 +486,8 @@ public:
     void enable() override;
     void disable() override;
 
-    // Window-field: the dynamic rows live inside a single container panel (assigned to the base
+    // The rows live in one container panel (the base `window`), so the field exposes a window instead
-    // `window` in BUILD), so the field exposes one window instead of a bare sizer. This lets focus/
+    // of a bare sizer and focus, sizing and teardown apply to the whole field.
-    // scroll, full-width sizing and teardown operate on the whole field.
     wxWindow* getWindow() override { return window; }
 
     void msw_rescale() override;
@@ -521,11 +519,10 @@ private:
     std::function<std::string()> m_selector;
 };
 
-// A settings row whose value is a raw JSON document that nobody should type by hand: the row shows a
+// A settings row whose value is a raw JSON document nobody types by hand: the button opens
-// button, the button opens PluginsConfigDialog, and the document it hands back becomes the field's
+// PluginsConfigDialog and the document it hands back becomes the field's value. The edit goes through
-// value. Routing the edit through the ordinary Field value/on_change_field path is what earns the row
+// the ordinary Field value/on_change_field path, so the row gets the same dirty state and revert arrow
-// the same dirty state and revert arrow as every other setting — the dialog itself never touches the
+// as any other setting — the dialog never touches the preset.
-// preset.
 class PluginConfigField : public Field {
     using Field::Field;
 public:
@@ -535,9 +532,8 @@ public:
 
     void BUILD() override;
 
-    // Which preset's capabilities the dialog lists. Supplied by the option group, which already
+    // Which preset's capabilities the dialog lists; set by the option group. An int for the same
-    // carries its Tab's type; an int for the same reason OptionsGroup::m_config_type is one — it
+    // reason OptionsGroup::m_config_type is one: it keeps Preset.hpp out of this header.
-    // keeps Preset.hpp out of this header.
     void set_preset_type(int type) { m_preset_type = type; }
 
     void set_value(const boost::any& value, bool change_event = false) override;
@@ -546,8 +542,8 @@ public:
     void enable() override;
     void disable() override;
 
-    // Window-field: the button is the whole field, so it is the window the option group sizes and
+    // The button is the whole field, so it is the window the option group sizes and positions (the
-    // positions (the ColourPicker idiom). A container panel would be sized but never laid out.
+    // ColourPicker idiom). A container panel would be sized but never laid out, collapsing the row.
     wxWindow* getWindow() override { return window; }
 
     void msw_rescale() override;

src/slic3r/GUI/OptionsGroup.cpp

@@ -127,9 +127,8 @@ const t_field& OptionsGroup::build_field(const t_config_option_key& id, const Co
         });
     }
 
-    // The dialog behind the button edits one preset's overrides, so it has to know which preset. The
+    // The dialog behind the button edits one preset's overrides, so it has to know which preset. Set
-    // group already carries its Tab's type, and fields are built lazily on activate() — too late for
+    // here because fields are built lazily on activate() — too late for the Tab to reach in afterwards.
-    // the Tab to reach in and set it afterwards.
     if (auto plugin_config_field = dynamic_cast<PluginConfigField*>(field.get()))
         if (auto config_group = dynamic_cast<ConfigOptionsGroup*>(this))
             plugin_config_field->set_preset_type(config_group->config_type());
@@ -717,9 +716,8 @@ std::string OptionsGroup::pick_plugin(const ConfigOptionDef& opt)
     if (selection.plugin_key.empty() || selection.name.empty())
         return {};
 
-    // Validate that the selected capability's plugin is resolvable, but only the bare capability
+    // Only the bare capability name is stored; the full "name;uuid;capability" reference is derived when
-    // name is stored in the option; the full "name;uuid;capability" reference is derived lazily
+    // the preset is serialized (see ConfigBase::save_plugin_collection). Resolve here to reject bad picks.
-    // when the preset is serialized (see ConfigBase::save_plugin_collection).
     Slic3r::PluginDescriptor descriptor;
     if (!manager.get_catalog().try_get_plugin_descriptor(selection.plugin_key, descriptor) || descriptor.name.empty())
         return {};

src/slic3r/GUI/PluginsConfigDialog.cpp

@@ -33,8 +33,8 @@ PluginsConfigDialog::PluginsConfigDialog(wxWindow* parent, Preset::Type type, co
     : WebViewHostDialog(parent, wxID_ANY, preset_type_title(type))
     , m_type(type)
 {
-    // Unparseable text is kept, not replaced: parse_plugin_overrides leaves the document empty and
+    // On failure the document stays empty and every row goes read-only (see m_parse_error), so a preset
-    // every row goes read-only, so a preset we cannot understand is never silently overwritten.
+    // we cannot understand is never silently overwritten.
     if (!parse_plugin_overrides(overrides_json, m_overrides, m_parse_error))
         BOOST_LOG_TRIVIAL(error) << "Plugins Config dialog: " << m_parse_error;
 
@@ -103,9 +103,8 @@ void PluginsConfigDialog::on_script_message(const nlohmann::json& payload)
         send_capability_config(id);
         show_status(_L("Configuration updated. Save the preset to persist it."), "success");
     } else if (command == "remove_preset_override") {
-        // "Restore defaults" for a preset means holding no override at all: the capability goes back
+        // "Restore defaults" for a preset means holding no override at all: the capability falls back
-        // to the global configuration, which is where an untouched preset takes its values from. The
+        // to the global configuration, not to the plugin's own get_default_config().
-        // plugin's own get_default_config() is the global config's default, not the preset's.
         if (!m_parse_error.empty()) {
             send_save_error(id, m_parse_error);
             return;

src/slic3r/GUI/PluginsConfigDialog.hpp

@@ -8,16 +8,12 @@
 
 namespace Slic3r { namespace GUI {
 
-// The config half of the Plugins dialog, scoped to one preset instead of one plugin: it lists the
+// Lists the plugin capabilities the edited preset of `m_type` uses (see capabilities_in_use) and edits
-// capabilities the edited preset of `m_type` actually uses (see capabilities_in_use) and edits each
+// each one's config, falling back to the global config where the preset has no override.
-// one's stored config, falling back to the global config where the preset has no override.
 //
-// It is a pure editor over a JSON document: it never writes to the preset and never writes to the
+// A pure editor over a JSON document: it never writes to the preset and never writes to the base config
-// base config file. The caller seeds it with the preset's raw override text and reads the edited
+// file. The caller seeds it with the preset's raw override text and reads the edited text back from
-// text back from overrides_json(). PluginConfigField, which owns the value, then feeds that through
+// overrides_json(); PluginConfigField owns the value and feeds it through the normal field/dirty pipeline.
-// the normal field/dirty pipeline — which is what makes the revert arrow behave like any other
-// setting. The capability list and config payloads are shared with PluginsDialog's Config tab
-// through PluginConfig's statics.
 class PluginsConfigDialog : public WebViewHostDialog
 {
 public:

src/slic3r/GUI/PluginsDialog.cpp

@@ -66,10 +66,8 @@ struct PluginCapabilityView
     bool enabled    = false;
     bool can_toggle = false;
     bool can_run    = false;
-    // Whether the capability supplies its own config UI, cached on the loaded capability at load
+    // Whether the capability supplies its own config UI; every capability is configurable, this only
-    // time. False for the descriptor-only rows shown for a plugin that is not loaded: its
+    // picks the editor. False for descriptor-only rows: an unloaded plugin has no capabilities yet.
-    // capabilities have not been materialized yet, so there is nothing to configure until it is
-    // activated. Every real capability is configurable; this only picks the editor.
     bool has_config_ui = false;
 };
 
@@ -83,7 +81,6 @@ struct PluginChangelogView
 // View-model for one plugin row in the dialog
 struct PluginDialogItem
 {
-    // Identity and display text
     std::string plugin_key;
     std::string plugin_id;
     std::string display_name;
@@ -100,7 +97,6 @@ struct PluginDialogItem
     std::vector<std::string> type_labels;
     std::vector<PluginChangelogView> changelog;
 
-    // Derived UI state
     PluginSource source              = PluginSource::Local;
     PluginStatus status              = PluginStatus::Inactive;
     PluginUpdateStatus update_status = PluginUpdateStatus::Normal;
@@ -109,7 +105,6 @@ struct PluginDialogItem
     bool is_loaded    = false;
     bool loading   = false;
 
-    // Installation and capability flags
     bool is_cloud_plugin       = false;
     bool has_local_package     = false;
     bool unauthorized          = false;
@@ -119,7 +114,6 @@ struct PluginDialogItem
     // Runtime capabilities in registration order, or descriptor-only type rows when unloaded.
     std::vector<PluginCapabilityView> capabilities;
 
-    // Row-level actions
     PluginAvailableActions available_actions;
 };
 
@@ -154,9 +148,8 @@ PluginDescriptor as_cloud_only_descriptor(PluginDescriptor descriptor)
     return descriptor;
 }
 
-// rescan_plugins() clears the whole catalog and rediscovers only local packages. When
+// rescan_plugins() clears the whole catalog and rediscovers only local packages, so when no fresh cloud
-// callers do not need fresh cloud data, reuse the current cloud rows after the local
+// data is needed, restore the current cloud rows afterwards or cloud-only rows would disappear.
-// rescan so cloud-only rows and cloud-derived UI state do not disappear.
 void refresh_plugin_catalog_blocking(bool fetch_cloud)
 {
     PluginManager& manager = PluginManager::instance();
@@ -220,10 +213,9 @@ nlohmann::json build_plugin_payload_item(const PluginDialogItem& dialog_item)
     }
     payload_item["capabilities"] = std::move(caps);
 
-    // The Config tab's sidebar, built by the shared builder so it stays identical to
+    // The Config tab's sidebar, built by the shared builder so it stays identical to PluginsConfigDialog's.
-    // PluginsConfigDialog's. Deliberately not the `capabilities` array above: that one is the list
+    // Not the `capabilities` array above: that is the list tab's, with enable/run state and descriptor-only
-    // tab's, carrying enable/run state and descriptor-only rows for plugins that are not activated
+    // rows the config view has no use for.
-    // yet — neither of which the config view has any use for.
     std::vector<PluginCapabilityIdentifier> config_ids;
     for (const PluginCapabilityView& capability : dialog_item.capabilities)
         if (!capability.name.empty())
@@ -329,20 +321,17 @@ PluginDialogItem build_plugin_dialog_item(const PluginDescriptor& descriptor)
                                  (descriptor.description.empty() ? "No description." : descriptor.description);
     item.author            = descriptor.author;
     item.version           = descriptor.version;
-    // Installed version: the cloud merge overwrites `version` with the latest cloud version, so prefer
+    // The cloud merge overwrites `version` with the latest cloud version, so prefer the preserved local
-    // the preserved local version, falling back to `version` for local-only / pre-merge descriptors.
+    // one, falling back to `version` for local-only / pre-merge descriptors.
     item.installed_version = descriptor.has_local_package() ?
                                  (descriptor.installed_version.empty() ? descriptor.version : descriptor.installed_version) :
                                  std::string{};
     item.latest_version    = descriptor.latest_available_version();
-    // why: sort by the same version the row displays (GetDisplayVersion in index.js) - installed when
-    //   installed, otherwise latest - so the Version sort matches what the user sees.
     item.sort_version      = item.installed_version.empty() ? item.latest_version : item.installed_version;
     item.type_label        = descriptor.type_label();
     item.type_key          = plugin_capability_type_to_string(descriptor.primary_capability_type());
-    // "types" is the display-only compatibility list. Cloud plugins show the raw labels the
+    // "types" is display-only: cloud plugins show the raw labels the service returned (which may not map
-    // service returned (which may not map to real capability types); local plugins derive them
+    // to real capability types); local plugins derive them from the capabilities actually discovered.
-    // from the capabilities actually discovered/loaded.
     if (descriptor.is_cloud_plugin() && !descriptor.display_types.empty()) {
         for (const std::string& label : descriptor.display_types)
             if (std::find(item.type_labels.begin(), item.type_labels.end(), label) == item.type_labels.end())
@@ -506,10 +495,8 @@ void PluginsDialog::on_script_message(const nlohmann::json& payload)
 
     const std::string command = payload.value("command", "");
     if (command == "request_plugins") {
-        // The web page finished loading and is asking for the current catalog. Plugin
+        // Discovery already runs at startup and on login, so the shared catalog is up to date by the
-        // discovery already runs at startup and on login, so the shared catalog is up to
+        // time the dialog opens: render it without a blocking fetch. Refresh is what rediscovers.
-        // date by the time the dialog opens. Just render it here - no blocking fetch on
-        // open. The Refresh button (refresh_plugins) is what triggers a fresh discovery.
         send_plugins();
     } else if (command == "refresh_plugins") {
         refresh_plugins();
@@ -588,7 +575,6 @@ nlohmann::json PluginsDialog::build_plugins_payload() const
     for (const PluginDescriptor& row : invalid)
         items.push_back(build_plugin_dialog_item(row));
 
-    // In-place sort
     sort_plugin_items_for_dialog(items, m_plugin_sort_key, m_plugin_sort_order);
 
     for (const PluginDialogItem& item : items)
@@ -635,9 +621,7 @@ void PluginsDialog::toggle_plugin(const std::string& plugin_key, bool enabled)
 
     PluginDescriptor row_data;
     if (!get_descriptor(plugin_key, row_data)) {
-        // The row no longer maps to a catalog entry (the catalog changed under the UI).
+        // The catalog changed under the UI. Toggling is local, so just re-render from it.
-        // Toggling is a local action, so just re-render from the current catalog; a cloud
-        // fetch (or clearing rescan) adds nothing here.
         send_plugins();
         return;
     }
@@ -663,7 +647,6 @@ void PluginsDialog::toggle_plugin(const std::string& plugin_key, bool enabled)
         return;
     }
 
-    // Check for capabilities that are currently in use
     auto loaded_capabilities = manager.get_loader().get_loaded_plugin_capabilities(plugin_key);
 
     if (!available_actions.can_toggle) {
@@ -684,8 +667,7 @@ void PluginsDialog::toggle_plugin(const std::string& plugin_key, bool enabled)
         }
 
         BOOST_LOG_TRIVIAL(info) << "Cloud plugin installed locally from Plugins dialog: " << plugin_key;
-        // download_and_install_cloud_plugin updates the descriptor with the installed
+        // download_and_install_cloud_plugin already updated the descriptor, so no rescan is needed here.
-        // local package state, so no rescan or cloud fetch is needed before loading.
         if (!get_descriptor(plugin_key, row_data)) {
             send_plugins();
             return;
@@ -731,9 +713,7 @@ void PluginsDialog::toggle_plugin_capability(const std::string& plugin_key, Plug
 
     PluginDescriptor row_data;
     if (!get_descriptor(plugin_key, row_data)) {
-        // The row no longer maps to a catalog entry (the catalog changed under the UI).
+        // The catalog changed under the UI. Toggling is local, so just re-render from it.
-        // Toggling is a local action, so just re-render from the current catalog; a cloud
-        // fetch (or clearing rescan) adds nothing here.
         send_plugins();
         return;
     }
@@ -950,9 +930,8 @@ void PluginsDialog::restore_capability_config(const std::string& plugin_key,
                                               PluginCapabilityType type,
                                               const std::string& capability_name)
 {
-    // Discards whatever the user had stored, so confirm first — same as the other destructive
+    // Destructive, so confirm first. The confirmation stays here rather than in PluginConfig: it needs
-    // actions in this dialog. The confirmation stays here rather than in PluginConfig: it needs a
+    // a parent window.
-    // parent window, and it is the dialog's business to ask.
     const int rc = wxMessageBox(wxString::Format(_L("Restore the default configuration for \"%s\"?\n\n"
                                                     "This discards the settings currently saved for this capability."),
                                                  from_u8(capability_name)),
@@ -1013,8 +992,8 @@ void PluginsDialog::run_script_plugin(const std::string& plugin_key, const std::
 
         send_plugins();
 
-        // The row now shows an "Error" status and the Diagnostics tab holds the full text, so surface
+        // The row already shows "Error" and the Diagnostics tab holds the full text, so report in the
-        // the outcome in the footer status bar instead of a modal box (prefer the friendlier override).
+        // footer status bar instead of a modal box.
         const wxString message = status_message.empty() ? from_u8(normalized_error) : status_message;
         show_status(message, "error");
     };
@@ -1063,13 +1042,11 @@ void PluginsDialog::run_script_plugin(const std::string& plugin_key, const std::
     std::string error;
     ExecutionResult result;
 
-    // Script plugins run on the main/UI thread (not a worker). They hold live, non-owning
+    // Script plugins run on the main/UI thread, not a worker: they hold live, non-owning
-    // ModelObject*/ModelVolume*/ModelInstance* aliases into host data and can mint ObjectIDs,
+    // ModelObject*/ModelVolume*/ModelInstance* aliases into host data and can mint ObjectIDs, which
-    // which libslic3r requires on the main thread (ObjectID.hpp's non-atomic s_last_id). Running
+    // libslic3r requires on the main thread (ObjectID.hpp's non-atomic s_last_id). The trade-off is that
-    // here makes those reads/instantiations legal and means nothing mutates the model underneath
+    // a slow execute() freezes the UI, so plugins must keep execute() quick and offload heavy work to
-    // a run. The trade-off is that a slow execute() freezes the UI, so the contract is to keep
+    // their own threading.Thread.
-    // execute() quick and offload heavy work to the plugin's own threading.Thread. orca.host.ui
-    // calls already no-op their main-thread marshaling here.
     {
         wxBusyCursor busy;
         try {
@@ -1098,7 +1075,6 @@ void PluginsDialog::run_script_plugin(const std::string& plugin_key, const std::
     if (failed) {
         plugin.reset();
         cap.reset();
-        // complete_with_error normalizes an empty message to "Script plugin failed." and reports via the status bar.
         complete_with_error(result.message, wxString());
         return;
     }
@@ -1122,9 +1098,8 @@ void PluginsDialog::update_plugin(const std::string& plugin_key)
     PluginDescriptor descriptor;
     const wxString name = get_descriptor(plugin_key, descriptor) ? from_u8(descriptor.name) : from_u8(plugin_key);
 
-    // update_cloud_plugin unloads the old plugin, deletes its local package, then downloads and
+    // update_cloud_plugin unloads the old plugin, deletes its local package and reinstalls the latest
-    // reinstalls the latest version. Each of those steps already runs off the main thread in the
+    // version; all of that is off-main-thread work, so run it on the worker behind a progress dialog.
-    // delete/install paths, so run the whole operation on the worker and pump a progress dialog.
     std::string error;
     bool updated = false;
     try {
@@ -1144,8 +1119,7 @@ void PluginsDialog::update_plugin(const std::string& plugin_key)
         return;
     }
 
-    // update_cloud_plugin installs the package and updates the in-memory descriptor
+    // update_cloud_plugin already updated the in-memory descriptor, so a UI refresh is enough here.
-    // (installed=true, update_available=false) on success.
     send_plugins();
     show_status(wxString::Format(_L("Updated \"%s\"."), name), "success");
 }
@@ -1332,10 +1306,8 @@ void PluginsDialog::delete_mine_local_and_cloud_plugin(const std::string& plugin
         return;
     }
 
-    // delete_mine_local_and_cloud_plugin already updated the in-memory catalog
+    // delete_mine_local_and_cloud_plugin already updated the in-memory catalog (see
-    // (finalize_cloud_plugin_removal removes the row and, when a local package existed,
+    // finalize_cloud_plugin_removal), so a UI refresh is sufficient here.
-    // re-syncs the cloud list itself), so a UI refresh is sufficient here - an extra
-    // clearing rescan + cloud fetch would be redundant.
     send_plugins();
     show_status(wxString::Format(_L("Deleted \"%s\"."), plugin_name), "success");
 }

src/slic3r/GUI/Tab.cpp

@@ -1790,9 +1790,8 @@ void Tab::on_value_change(const std::string& opt_key, const boost::any& value)
         return;
     }
 
-    // Keep this preset's "plugins" manifest in sync when a plugin picker changes, so the edited preset
+    // Keep this preset's "plugins" manifest in sync when a plugin picker changes, so full_config() and
-    // always carries resolved "name;uuid;capability" references that full_config() and save_to_json()
+    // save_to_json() always find resolved "name;uuid;capability" references and rebuild it nowhere else.
-    // then pass downstream as-is -- no separate rebuild anywhere else.
     if (const ConfigOptionDef* opt_def = m_config->def()->get(opt_key);
         opt_def && opt_def->is_plugin_backed())
         m_config->update_plugin_manifest();
@@ -3086,10 +3085,8 @@ void TabPrint::build()
         option.opt.full_width = true;
         optgroup->append_single_option_line(option, "others_settings_plugin_picker");
 
-        // Its own group rather than the one above: that one hides its labels, and this row needs its
+        // Its own group: the one above hides its labels, and this row needs its label — and the revert
-        // label — and so the revert arrow beside it — to show. No label-width override either: a 0
+        // arrow beside it — to show. No label-width override either, as a 0 there means "no label column".
-        // there means "no label column", which is what the neighbouring full-width groups want and
-        // what would collapse this one.
         optgroup = page->new_optgroup(L("Plugin Preferences"), L"param_gcode");
         optgroup->append_single_option_line("plugin_preference_overrides");
 

src/slic3r/plugin/PluginConfig.cpp

@@ -20,9 +20,8 @@ namespace Slic3r {
 
 namespace {
 
-// The version of the plugin package currently running. PluginDescriptor::version is
+// PluginDescriptor::version is overwritten with the latest cloud version on a cloud merge, so it can
-// overwritten with the latest cloud version when a cloud merge happens, so it can name a
+// name a version that is not the one on disk; installed_version is what actually loaded.
-// version that is not the one on disk; installed_version is what actually loaded.
 std::string running_plugin_version(const std::string& plugin_key)
 {
     PluginDescriptor descriptor;
@@ -31,10 +30,9 @@ std::string running_plugin_version(const std::string& plugin_key)
     return descriptor.installed_version.empty() ? descriptor.version : descriptor.installed_version;
 }
 
-// The identity a capability is allowed to address: its own. PluginLoader stamps both halves
+// PluginLoader stamps both halves onto the instance when it materializes the capability, so the
-// onto the instance when it materializes the capability, so the caller never supplies them
+// caller never supplies them and cannot name another capability's entry. Empty means the instance
-// and cannot name another capability's entry. Empty means the instance was never materialized
+// was never materialized: refuse rather than read or clobber a wrong entry.
-// (so it has no config to address) and we refuse rather than read or clobber a wrong entry.
 std::pair<std::string, std::string> capability_identity(const PluginCapabilityInterface& capability, const char* api_name)
 {
     std::pair<std::string, std::string> id{capability.audit_plugin_key(), capability.audit_capability_name()};
@@ -44,11 +42,11 @@ std::pair<std::string, std::string> capability_identity(const PluginCapabilityIn
     return id;
 }
 
-// The identity above, completed with the type, which decides which preset may override the
+// The identity above plus the type, which decides which preset may override the capability (see
-// capability (see preset_type_for_capability). Taken from the instance rather than from the loader's
+// preset_type_for_capability). Taken from the instance, not the loader's registry: a capability
-// registry: a capability calling get_config() from on_load() is not registered yet, and it must
+// calling get_config() from on_load() is not registered yet and must still see its preset's config.
-// still see its preset's config. get_type() is the plugin's own method, so a raising one costs it
+// A raising get_type() costs it only the preset layer — Unknown names no preset, so the base config
-// only the preset layer — Unknown names no preset, and the base config answers as it always did.
+// answers.
 PluginCapabilityIdentifier capability_full_identity(const PluginCapabilityInterface& capability, const char* api_name)
 {
     const auto [plugin_key, capability_name] = capability_identity(capability, api_name);
@@ -113,8 +111,8 @@ bool PluginConfig::save()
         return false;
     }
 
-    // Write to a PID-suffixed file and rename it into place, so a crash mid-write cannot
+    // Write to a PID-suffixed file and rename it into place, so a crash mid-write cannot truncate an
-    // truncate an existing config. Same approach as AppConfig::save().
+    // existing config. Same approach as AppConfig::save().
     const std::string path_pid = (boost::format("%1%.%2%") % path % get_current_pid()).str();
 
     boost::nowide::ofstream file;
@@ -198,18 +196,16 @@ bool PluginConfig::dirty() const
 
 nlohmann::json capability_get_config(const PluginCapabilityInterface& capability)
 {
-    // The active preset's override, when it has one, is the config this run must use: it is what the
+    // Shares its resolution with the dialogs, so a capability reads back exactly the config its UI
-    // user attached to the preset being sliced, and config.json is the fallback. The resolution is
+    // showed as effective. Stored in neither layer: an empty object, so a plugin can index it
-    // shared with the dialogs, so a capability reads back exactly the config its UI showed as
+    // unconditionally.
-    // effective. Stored in neither layer: an empty object, so a plugin can index it unconditionally.
     return active_capability_config(capability_full_identity(capability, "get_config")).config;
 }
 
 std::string capability_get_config_version(const PluginCapabilityInterface& capability)
 {
-    // The version that wrote the config get_config() just handed out, whichever layer that was: the
+    // Must resolve through the same layer as get_config(), or a plugin would migrate one layer's
-    // two must come from the same layer, or a plugin would migrate one layer's config by another's
+    // config by another's version stamp.
-    // version stamp.
     return active_capability_config(capability_full_identity(capability, "get_config_version")).stored_plugin_version;
 }
 
@@ -226,8 +222,7 @@ nlohmann::json PluginConfig::capabilities_payload(const std::vector<PluginCapabi
 
     nlohmann::json payload = nlohmann::json::array();
     for (const PluginCapabilityIdentifier& id : caps) {
-        // Read has_config_ui off the live capability rather than trusting the caller's copy: a
+        // A capability unloaded since the list was built has nothing to configure.
-        // capability that has been unloaded since the list was built has nothing to configure.
         const auto capability = loader.get_plugin_capability_by_name(id);
         if (!capability)
             continue;
@@ -243,9 +238,8 @@ nlohmann::json PluginConfig::capabilities_payload(const std::vector<PluginCapabi
     return payload;
 }
 
-// Replies with one capability's stored config, plus the custom HTML UI when the capability provides
+// Config is sent as a JSON value, not text: the default editor pretty-prints it into its textarea,
-// one. Config is sent as a JSON value, not text: the default editor pretty-prints it into its
+// and a custom UI receives it as-is through window.orca.
-// textarea, and a custom UI receives it as-is through window.orca.
 nlohmann::json PluginConfig::get_config_response(const PluginCapabilityIdentifier& id)
 {
     nlohmann::json response;
@@ -258,7 +252,7 @@ nlohmann::json PluginConfig::get_config_response(const PluginCapabilityIdentifie
     response["error"]           = "";
 
     // Scoped to the full identity, so a stale request from a page that has not caught up with a
-    // refresh cannot read a different plugin's config — it just misses.
+    // refresh misses rather than reading a different plugin's config.
     const auto cap = PluginManager::instance().get_loader().get_plugin_capability_by_name(id);
     if (!cap) {
         BOOST_LOG_TRIVIAL(warning) << "Ignoring config request for a capability that is no longer loaded. plugin_key="
@@ -270,9 +264,8 @@ nlohmann::json PluginConfig::get_config_response(const PluginCapabilityIdentifie
     response["config"] = PluginManager::instance().get_config().get_config(id.plugin_key, id.name).config;
 
     if (cap->has_config_ui) {
-        // Plugin-authored HTML. A raising or empty get_config_ui() costs the capability only its
+        // A raising or empty get_config_ui() costs the capability only its custom UI: report the
-        // custom UI: we report the failure and let the page fall back to the default JSON editor,
+        // failure and let the page fall back to the default JSON editor over the same stored config.
-        // which edits the very same stored config.
         std::string html;
         std::string error;
         {
@@ -324,8 +317,8 @@ nlohmann::json PluginConfig::save_config_response(const PluginCapabilityIdentifi
 
     nlohmann::json parsed = config;
     if (config.is_string()) {
-        // The page validates as the user types, but it is not the authority: re-parse here so a
+        // The page validates as the user types, but it is not the authority: re-parse so a malformed
-        // malformed document is rejected before it can reach config.json.
+        // document is rejected before it can reach config.json.
         parsed = nlohmann::json::parse(config.get<std::string>(), nullptr, /* allow_exceptions */ false);
         if (parsed.is_discarded()) {
             response["error"] = GUI::into_u8(_L("The configuration is not valid JSON. Your changes were not saved."));
@@ -342,15 +335,14 @@ nlohmann::json PluginConfig::save_config_response(const PluginCapabilityIdentifi
 
     BOOST_LOG_TRIVIAL(info) << "Saved plugin capability config. plugin_key=" << id.plugin_key << " capability_name=" << id.name;
 
-    // Echo the persisted value back so the editor reloads from what is actually stored rather than
+    // Echo back what was persisted, not what the user typed, so the editor reloads from the store.
-    // from what the user typed.
     response["ok"]     = true;
     response["config"] = PluginManager::instance().get_config().get_config(id.plugin_key, id.name).config;
     return response;
 }
 
-// The host does not invent the default: a capability that does not override get_default_config()
+// The host never invents the default: a capability that does not override get_default_config()
-// restores an empty config, which is exactly right for one that applies its own defaults on read.
+// restores an empty config, which is right for one that applies its own defaults on read.
 nlohmann::json PluginConfig::restore_config_response(const PluginCapabilityIdentifier& id)
 {
     nlohmann::json response;
@@ -383,8 +375,8 @@ nlohmann::json PluginConfig::restore_config_response(const PluginCapabilityIdent
         }
     }
 
-    // A raising hook leaves the stored config exactly as it was: better to restore nothing than to
+    // A raising hook leaves the stored config as it was: better to restore nothing than to wipe the
-    // wipe the user's settings on the strength of a broken plugin.
+    // user's settings on the strength of a broken plugin.
     if (!error.empty()) {
         BOOST_LOG_TRIVIAL(error) << "Plugin capability get_default_config() failed. plugin_key=" << id.plugin_key
                                  << " capability_name=" << id.name << " error=" << error;
@@ -404,7 +396,6 @@ nlohmann::json PluginConfig::restore_config_response(const PluginCapabilityIdent
     BOOST_LOG_TRIVIAL(info) << "Restored default plugin capability config. plugin_key=" << id.plugin_key
                             << " capability_name=" << id.name;
 
-    // Reuses the saved reply, so both editors reload from what was actually persisted.
     response["ok"]     = true;
     response["config"] = PluginManager::instance().get_config().get_config(id.plugin_key, id.name).config;
     return response;

src/slic3r/plugin/PluginConfig.hpp

@@ -32,32 +32,8 @@ Example config.json shape
             "plugin_key": "some_name",
             "capability": "capability_name",
             "plugin_version": "1.0.0",
-            "cap_config": {
+            "cap_config": { "plugin": "specific", "stuff": "here" }
-                "some": "plugin",
+        }
-                "capability": "specific",
-                "stuff": "here"
-            }
-        },
-        {
-            "plugin_key": "some_name",
-            "capability": "capability_name",
-            "plugin_version": "1.0.0",
-            "cap_config": {
-                "some": "plugin",
-                "capability": "specific",
-                "stuff": "here"
-            }
-        },
-        {
-            "plugin_key": "some_name",
-            "capability": "capability_name",
-            "plugin_version": "1.0.0",
-            "cap_config": {
-                "some": "plugin",
-                "capability": "specific",
-                "stuff": "here"
-            }
-        },
     ]
 }
 */
@@ -73,14 +49,12 @@ struct BaseConfig {
     bool empty() const { return plugin_key.empty() || capability_name.empty(); }
 };
 
-// Consolidated store for every plugin capability's configuration, persisted as a single
+// Store for every plugin capability's configuration, persisted as a single config.json alongside the
-// config.json alongside the installed plugins. The shape of `cap_config` belongs to the
+// installed plugins. The shape of `cap_config` belongs to the plugin; this class only round-trips it.
-// plugin; this class only round-trips it.
 //
-// A capability is identified by (plugin_key, capability_name). `plugin_version` is metadata
+// A capability is identified by (plugin_key, capability_name). `plugin_version` records which version
-// recording which version last wrote the entry, letting an upgraded plugin spot a stale
+// last wrote the entry, so an upgraded plugin can spot a stale config and migrate it. It is
-// config and migrate it. Version is deliberately not part of the identity, so upgrading a
+// deliberately not part of the identity: upgrading a plugin must not reset the user's settings.
-// plugin does not silently reset the user's settings.
 //
 // Plugin code runs on worker threads, so every entry point is mutex-guarded.
 class PluginConfig
@@ -88,51 +62,42 @@ class PluginConfig
 public:
     static const std::string plugin_config_file() { return (boost::filesystem::path(get_orca_plugins_dir()) / PLUGIN_CONFIG_DIR).string(); }
 
-    // Replaces the in-memory store with what is on disk. A missing or malformed file leaves
+    // A missing or malformed file leaves the store empty rather than throwing: a bad plugin config
-    // the store empty rather than throwing: a bad plugin config must not block startup.
+    // must not block startup.
     void load();
 
-    // Rewrites config.json atomically. Clears the dirty flag only once the file is in place.
+    // Rewrites config.json atomically. False means the config on disk is unchanged.
-    // False means the config on disk is unchanged.
     bool save();
 
     void save_config(const std::string& plugin_key, const std::string& capability_name, const std::string& version, const nlohmann::json& config);
     void save_config(const BaseConfig& config);
 
-    // Replaces one capability's cap_config and writes config.json straight away, stamping the
+    // Replaces one capability's cap_config and writes config.json straight away, stamping the entry
-    // entry with the plugin version currently running. Every other entry is round-tripped
+    // with the plugin version currently running. Every other entry is round-tripped untouched, so
-    // untouched, so saving one capability cannot disturb another's config.
+    // saving one capability cannot disturb another's config.
-    // The single mutation entry point for both the Plugins dialog and the Python binding.
     bool store_capability_config(const std::string& plugin_key, const std::string& capability_name, const nlohmann::json& config);
     bool erase_capability_config(const std::string& plugin_key, const std::string& capability_name);
 
-    // Returns a default-constructed BaseConfig (see BaseConfig::empty) when the capability has
+    // A default-constructed BaseConfig (see BaseConfig::empty) when there is no stored config.
-    // no stored config.
     BaseConfig get_config(const std::string& plugin_key, const std::string& capability_name) const;
     bool has_config(const std::string& plugin_key, const std::string& capability_name) const;
 
     bool dirty() const;
 
     // ---- Webview-facing helpers, shared by PluginsDialog's Config tab and PluginsConfigDialog ----
-    //
+    // Static because a capability's config is addressed globally by (plugin_key, capability_name)
-    // These build the payloads both dialogs' config views speak, so the two pages stay in step and
+    // through PluginManager's store, not through any one PluginConfig instance. The caller owns the
-    // neither dialog owns the config protocol. They are static because a capability's config is
+    // UI: it confirms destructive restores and shows status toasts.
-    // addressed globally by (plugin_key, capability_name) through PluginManager's store, not through
-    // any one PluginConfig instance.
-    //
-    // The caller owns the UI: it confirms destructive restores and shows status toasts. These only
-    // touch the store, the loaded capability, and the payload.
 
-    // The config sidebar's rows: one entry per capability, in the order given. Capabilities that are
+    // The config sidebar's rows, in the order given. Capabilities that are no longer loaded are
-    // no longer loaded are skipped — the sidebar only offers what can actually be configured.
+    // skipped — the sidebar only offers what can actually be configured.
     static nlohmann::json capabilities_payload(const std::vector<PluginCapabilityIdentifier>& caps);
 
     // One capability's stored config, plus its custom HTML UI when it provides one.
     static nlohmann::json get_config_response(const PluginCapabilityIdentifier& id);
 
-    // Persists one capability's config. `config` is either text straight from the default editor
+    // Persists one capability's config. `config` is either text from the default editor (re-parsed
-    // (re-parsed here, so malformed JSON can never reach config.json) or a structured value from a
+    // here, so malformed JSON can never reach config.json) or a structured value from a custom UI.
-    // custom UI.
     static nlohmann::json save_config_response(const PluginCapabilityIdentifier& id, const nlohmann::json& config);
 
     // Overwrites one capability's stored config with its get_default_config(). The caller must have
@@ -145,23 +110,21 @@ private:
     bool m_dirty = false;
 };
 
-// Host implementations behind the capability-level Python config API (bound onto every
+// Host implementations behind the capability-level Python config API (bound onto every capability
-// capability class in PythonPluginBridge). The capability addresses only itself: the
+// class in PythonPluginBridge). The capability addresses only itself: the (plugin_key,
-// (plugin_key, capability_name) pair is read off the instance the call arrived on, never
+// capability_name) pair is read off the instance the call arrived on, never passed in from Python,
-// passed in from Python, so a capability cannot reach another capability's config.
+// so a capability cannot reach another capability's config. Throws std::runtime_error (RuntimeError
-// Throw std::runtime_error (surfacing to Python as RuntimeError) on an unmaterialized instance.
+// in Python) on an unmaterialized instance.
 
-// Only the user-editable cap_config, resolved the way the config UI presents it: the active preset's
+// The cap_config resolved as the config UI presents it: the active preset's override when it has
-// override when it has one, otherwise the config stored here (see active_capability_config). An
+// one, otherwise the config stored here (see active_capability_config). Empty object when neither.
-// empty object when neither layer holds one.
 nlohmann::json capability_get_config(const PluginCapabilityInterface& capability);
-// The plugin version that last wrote the config get_config() returns — the same layer it came from —
+// The plugin version that wrote the config get_config() returns — the same layer it came from — so a
-// so a plugin can migrate a stale cap_config. Empty when the capability has no stored config.
+// plugin can migrate a stale cap_config. Empty when the capability has no stored config.
 std::string capability_get_config_version(const PluginCapabilityInterface& capability);
-// Replaces cap_config and persists. Host-managed identity and version metadata are preserved.
+// Replaces cap_config and persists. Always writes the store, never a preset: presets are the user's
-// Writes the store here, never a preset: presets are the user's to edit, and a plugin saving from a
+// to edit, and a plugin saving from a worker thread cannot mark one dirty. A capability whose active
-// worker thread cannot mark one dirty. A capability whose active preset overrides it will therefore
+// preset overrides it will therefore keep reading that override back, not what it saved.
-// keep reading that override back, not what it saved.
 bool capability_save_config(const PluginCapabilityInterface& capability, const nlohmann::json& config);
 
 } // namespace Slic3r

src/slic3r/plugin/PluginResolver.cpp

@@ -28,18 +28,16 @@ namespace Slic3r {
 
 namespace {
 
-// Return the name of the tracked option in `preset` whose value references `ref`'s capability, or
+// The tracked option in `preset` whose value references `ref`'s capability, or "" when none does.
-// an empty string when no active option uses it. The result doubles as the "Jump to" target and as
+// Doubles as the "Jump to" target and as the signal that the plugin is still required: a missing
-// the signal that the plugin is still required: a missing plugin with no referencing option is
+// plugin with no referencing option is dropped from the set.
-// considered resolved and dropped from the missing set.
 std::string find_option_for_capability(Preset::Type type, const Preset& preset, const PluginCapabilityRef& ref)
 {
     if (type != Preset::TYPE_PRINT && type != Preset::TYPE_PRINTER && type != Preset::TYPE_FILAMENT)
         return {};
 
-    // Plugin-bearing options opt in via ConfigOptionDef::is_plugin_backed (a non-empty plugin_type),
+    // Options opt in via ConfigOptionDef::is_plugin_backed, so scan the definition rather than keep a
-    // so scan the preset's definition rather than maintaining a hardcoded per-type field list. A typed
+    // hardcoded per-type field list. A typed preset's config only holds keys for its own type.
-    // preset's config only contains keys for its own type, so this naturally stays scoped to `type`.
     const ConfigDef* def = preset.config.def();
     if (def == nullptr)
         return {};
@@ -80,8 +78,7 @@ std::string find_option_for_capability(Preset::Type type, const Preset& preset,
 
 } // namespace
 
-// One missing-plugin set per tracked preset type, keyed by the full "name;uuid;capability" ref.
+// One set per tracked preset type, keyed by the full "name;uuid;capability" ref.
-// Only TYPE_PRINT (process), TYPE_PRINTER (machine) and TYPE_FILAMENT are tracked.
 static std::map<Preset::Type, std::unordered_map<std::string, MissingPlugin>> s_missing;
 static std::mutex s_missing_mutex;
 // Installed-but-inactive capabilities (not loaded, or loaded-but-disabled); resolvable locally.
@@ -108,7 +105,6 @@ std::vector<PluginCapabilityRef> referenced_capabilities(Preset::Type type, cons
         const auto ref = parse_capability_ref(entry);
         if (!ref)
             continue;
-        // The same test the missing-plugin path uses: an entry no option points at is not in use.
         if (find_option_for_capability(type, preset, *ref).empty())
             continue;
         refs.push_back(*ref);
@@ -119,15 +115,15 @@ std::vector<PluginCapabilityRef> referenced_capabilities(Preset::Type type, cons
 namespace {
 
 // Resolve one preset's referenced capabilities to loaded capability identifiers, appending to `out`.
-// A ref whose plugin is not in the catalog, or whose capability is not currently loaded, is dropped:
+// A ref not in the catalog, or whose capability is not loaded, is dropped: no instance means nothing
-// there is no instance to ask for a config UI or defaults, so there is nothing to configure.
+// to configure.
 void collect_capabilities_in_use(Preset::Type type, const Preset& preset, std::vector<PluginCapabilityIdentifier>& out)
 {
     PluginLoader&        loader  = PluginManager::instance().get_loader();
     const PluginCatalog& catalog = PluginManager::instance().get_catalog();
 
     for (const PluginCapabilityRef& ref : referenced_capabilities(type, preset)) {
-        // Cloud plugins resolve by UUID, local plugins by plugin_key — the rule refresh_missing_plugins uses.
+        // Cloud plugins resolve by UUID, local plugins by plugin_key.
         const std::string key = ref.uuid.empty() ? ref.name : ref.uuid;
         if (key.empty())
             continue;
@@ -156,8 +152,8 @@ std::vector<PluginCapabilityIdentifier> capabilities_in_use(const PresetBundle&
     } else if (type == Preset::TYPE_PRINTER) {
         collect_capabilities_in_use(type, preset_bundle.printers.get_edited_preset(), result);
     } else {
-        // Filament: every selected filament preset, tested against its own config. (Note that
+        // Each filament preset is tested against its own config; refresh_missing_plugins cannot do
-        // refresh_missing_plugins cannot do this — it unions the manifests and loses the preset.)
+        // that, as it unions the manifests and loses the preset.
         for (const std::string& filament_name : preset_bundle.filament_presets)
             if (const Preset* filament = preset_bundle.filaments.find_preset(filament_name))
                 collect_capabilities_in_use(type, *filament, result);
@@ -233,9 +229,7 @@ std::string resolve_recovery_url(const PluginCapabilityRef& ref)
     return resolve_cloud_base_url() + "/app/plugins/plugin-hub?search=" + Http::url_encode(ref.name);
 }
 
-// Reports whether the loaded plugin currently exposes the referenced capability (in any enabled
+// {present, enabled}; {false, false} when the plugin is not loaded or does not provide the capability.
-// state) and whether that capability is enabled. Returns {false, false} when the plugin is not
-// loaded or does not provide the capability.
 static std::pair<bool, bool> loaded_capability_state(const std::string& plugin_key, const PluginCapabilityRef& ref)
 {
     bool present = false, enabled = false;
@@ -312,7 +306,7 @@ void refresh_missing_plugins(Preset::Type type, const ConfigOptionStrings* manif
             continue;
 
         if (!installed) {
-            // Not on disk — needs download/install (existing behavior).
+            // Not on disk — needs download/install.
             std::string recovery_url = ref->uuid.empty() ? resolve_recovery_url(*ref) : std::string();
             missing_set.emplace(entry, MissingPlugin{*ref, std::move(recovery_url), std::move(opt), type, PluginCapabilityType::Unknown});
         } else if (!loaded || cap_present) {
@@ -406,7 +400,6 @@ static void report_install_failure(const std::string& message)
 
 void resolve_missing_plugins(const std::vector<std::string>& refs, PluginInstallProgress progress)
 {
-    // Collect the unique cloud UUIDs to install; local refs are handled via the browser flow.
     std::vector<std::string> uuids;
     for (const std::string& r : refs) {
         const auto ref = parse_capability_ref(r);
@@ -430,7 +423,6 @@ void resolve_missing_plugins(const std::vector<std::string>& refs, PluginInstall
 
             const std::string& uuid = uuids[i];
 
-            // Use a friendly name for the progress message when the catalog already knows it.
             std::string display_name = uuid;
             PluginDescriptor known;
             if (mgr.get_catalog().try_get_plugin_descriptor(uuid, known) && !known.name.empty())
@@ -466,8 +458,7 @@ void resolve_inactive_plugins(const std::vector<std::string>& refs)
     PluginManager& mgr     = PluginManager::instance();
     PluginCatalog& catalog = mgr.get_catalog();
 
-    // Group the requested capabilities by owning plugin so each plugin is loaded once with the full
+    // Group by owning plugin so each plugin is loaded once with the full set to enable.
-    // set to enable.
     std::map<std::string, std::vector<std::string>> by_plugin;
     for (const std::string& r : refs) {
         const auto ref = parse_capability_ref(r);
@@ -482,11 +473,9 @@ void resolve_inactive_plugins(const std::vector<std::string>& refs)
     if (by_plugin.empty())
         return;
 
-    // load_plugin loads+enables a not-loaded plugin (async) and enables the listed capabilities on an
+    // The fresh-load path does NOT fire the capability-load callback the GUI uses to clear the
-    // already-loaded one. The fresh-load path does NOT fire the capability-load callback the GUI uses
+    // notification, so wait for each load off the UI thread and re-validate once. That clears the
-    // to clear the notification, so wait for each load off the UI thread and then re-validate once —
+    // inactive notification, or flips it to broken if the plugin does not provide the capability.
-    // mirroring the cloud-install flow. This clears the inactive notification, or flips it to broken
-    // if the loaded plugin turns out not to provide the capability.
     std::vector<std::pair<std::string, std::vector<std::string>>> work(by_plugin.begin(), by_plugin.end());
     std::thread([work = std::move(work)]() {
         PluginManager& mgr     = PluginManager::instance();
@@ -506,7 +495,6 @@ void resolve_inactive_plugins(const std::vector<std::string>& refs)
 
 void open_missing_plugins_on_cloud(const std::vector<std::string>& local_refs)
 {
-    // One missing plugin: deep-link a search for it. Multiple: just open the plugin hub.
     if (local_refs.size() == 1) {
         if (const auto ref = parse_capability_ref(local_refs.front())) {
             wxLaunchDefaultBrowser(GUI::from_u8(resolve_recovery_url(*ref)), wxBROWSER_NEW_WINDOW);

src/slic3r/plugin/PluginResolver.hpp

@@ -24,10 +24,9 @@ struct MissingPlugin
     PluginCapabilityType type{PluginCapabilityType::Unknown};
 };
 
-// Rebuild the missing-plugin set owned by a single preset type from that preset's "plugins"
+// Rebuild one preset type's missing-plugin set from its "plugins" manifest, comparing each ref
-// manifest, comparing each ref against the live plugin catalog and loaded/enabled capabilities. A
+// against the live catalog and loaded/enabled capabilities. A null/empty manifest clears the set.
-// null/empty manifest clears the set for that type. Only TYPE_PRINT (process), TYPE_PRINTER
+// Only TYPE_PRINT, TYPE_PRINTER and TYPE_FILAMENT are tracked.
-// (machine) and TYPE_FILAMENT are tracked; other types are ignored.
 void refresh_missing_plugins(Preset::Type type, const ConfigOptionStrings* manifest, const Preset* preset = nullptr);
 void refresh_missing_plugins(const PresetBundle& preset_bundle);
 
@@ -37,22 +36,16 @@ std::vector<MissingPlugin> get_missing_cloud_plugins();
 std::vector<MissingPlugin> get_missing_local_plugins();
 bool                     has_missing_plugins();
 
-// Installed-but-inactive capabilities: the plugin has a local package but the referenced capability
+// Installed-but-inactive: the plugin has a local package but is not loaded, or is loaded with the
-// is not active because the plugin is not loaded, or it is loaded but the capability is disabled.
+// capability disabled. Resolved locally by loading and/or enabling — no download.
-// Resolved locally by loading the plugin and/or enabling the capability — no download.
 std::vector<MissingPlugin> get_inactive_plugins();
 bool                     has_inactive_plugins();
 
 // Broken references: the plugin is installed AND loaded but does not provide the referenced
-// capability at all (renamed/removed/outdated plugin). Activation cannot fix these; surfaced as an
+// capability at all (renamed/removed/outdated plugin). Activation cannot fix these.
-// informational notification pointing the user at OrcaCloud to update the plugin.
 std::vector<MissingPlugin> get_broken_plugins();
 bool                     has_broken_plugins();
 
-// Resolution actions invoked from the missing-plugin notifications:
-// - cloud refs are subscribed/installed and loaded on a detached worker thread; failures are
-//   reported through a non-blocking notification. Non-cloud refs are ignored.
-
 // Optional progress hook for the cloud install worker. All three callbacks fire on the worker
 // thread; implementations must only touch thread-safe state or marshal to the UI thread.
 struct PluginInstallProgress
@@ -65,43 +58,39 @@ struct PluginInstallProgress
     std::function<void()> on_finished;
 };
 
-// Cloud refs only; local refs are handled via the browser flow. `progress` is optional — a
+// Subscribe, install and load the cloud refs on a detached worker; failures are reported through a
-// default-constructed value preserves the previous silent behavior.
+// non-blocking notification. Local refs are ignored — they go through the browser flow below.
 void resolve_missing_plugins(const std::vector<std::string>& refs,
                              PluginInstallProgress progress = {});
 
-// Activate inactive plugins: load each referenced plugin (passing the capabilities to enable) and/or
+// Load each referenced plugin and/or enable its disabled capabilities. Local only — no network. The
-// enable already-loaded-but-disabled capabilities. Local only — no network. The loads run on a
+// loads run on a background worker that waits for them and then re-validates the plate, clearing the
-// background worker that waits for them and then re-validates the plate, clearing the notification
+// notification (or reclassifying the ref as broken if the plugin does not provide the capability).
-// (or reclassifying the ref as broken if the loaded plugin turns out not to provide the capability).
 void resolve_inactive_plugins(const std::vector<std::string>& refs);
 
-// - local refs are opened on the OrcaCloud plugin hub (search when exactly one ref, hub otherwise).
+// Opens the OrcaCloud plugin hub (a search when there is exactly one ref, the hub otherwise).
 void open_missing_plugins_on_cloud(const std::vector<std::string>& local_refs);
 
 std::string create_full_ref(const PluginCapabilityRef& ref);
 std::string resolve_recovery_url(const PluginCapabilityRef& ref);
 
 // The capabilities `preset`'s "plugins" manifest declares AND that one of its plugin-backed options
-// (ConfigOptionDef::is_plugin_backed) currently references. A manifest entry nobody points at is not
+// (ConfigOptionDef::is_plugin_backed) currently references: a manifest entry nobody points at is not
-// in use. Pure preset logic — the plugin catalog and loader are not consulted. Empty for untracked
+// in use. Pure preset logic — the catalog and loader are not consulted. Empty for untracked types.
-// preset types.
 std::vector<PluginCapabilityRef> referenced_capabilities(Preset::Type type, const Preset& preset);
 std::vector<PluginCapabilityIdentifier> capabilities_in_use(Preset::Type type, const Preset& preset);
 
-// The preset type that owns capabilities of `type` — the one whose presets can reference them and
+// The preset type whose presets can reference capabilities of `type` and therefore carry their
-// therefore carry their overrides. Derived from the ConfigDef rather than hardcoded: a plugin-backed
+// overrides. Derived from the ConfigDef rather than hardcoded: a plugin-backed option names the
-// option names the capability type it accepts (ConfigOptionDef::plugin_type) and belongs to exactly
+// capability type it accepts (ConfigOptionDef::plugin_type) and belongs to exactly one preset type,
-// one preset type, so declaring the option is all it takes to map a new capability type.
+// so declaring the option is all it takes to map a new capability type. TYPE_INVALID when no option
-// TYPE_INVALID when no option accepts the type (nothing can reference it, so no preset owns it).
+// accepts the type — nothing can reference it, so no preset owns it.
 Preset::Type preset_type_for_capability(PluginCapabilityType type);
 
-// The capabilities the active preset(s) of `type` reference (see referenced_capabilities) and that
+// The referenced capabilities of the active preset(s) of `type` that are loaded right now: the set
-// are loaded right now: the set that can actually be configured. Missing and broken refs are absent,
+// that can actually be configured. Missing and broken refs are absent, having no instance to ask for
-// having no instance to ask for a config UI or defaults. A loaded-but-disabled capability IS listed —
+// a config UI or defaults. A loaded-but-disabled capability IS listed — it still has stored config
-// it still has stored config worth editing, and disabling it is not a reason to hide that.
+// worth editing.
-// TYPE_FILAMENT unions every selected filament preset; a capability used by two extruders is listed
-// once. Deduped on the full identity, so two plugins exposing a same-named capability stay distinct.
 std::vector<PluginCapabilityIdentifier> capabilities_in_use(const PresetBundle& preset_bundle, Preset::Type type);
 
 bool check_capability_in_use(const std::string& capability_refs);

src/slic3r/plugin/PresetPluginConfig.cpp

@@ -26,8 +26,8 @@ std::string running_plugin_version(const std::string& plugin_key)
     return descriptor.installed_version.empty() ? descriptor.version : descriptor.installed_version;
 }
 
-// Null wherever the plugin host runs without the GUI app (the unit tests): there are no presets
+// Null wherever the plugin host runs without the GUI app (the unit tests). wxGetApp() dereferences
-// then, only the base config. wxGetApp() dereferences the app unconditionally, so ask wxWidgets.
+// the app unconditionally, so ask wxWidgets instead.
 const PresetBundle* active_preset_bundle()
 {
     const auto* app = dynamic_cast<const GUI::GUI_App*>(wxApp::GetInstance());
@@ -46,22 +46,15 @@ const Preset* active_preset_for(Preset::Type type)
     case Preset::TYPE_PRINT: return &bundle->prints.get_edited_preset();
     case Preset::TYPE_PRINTER: return &bundle->printers.get_edited_preset();
 
-    // Deliberately unimplemented, not forgotten.
+    // Deliberately unimplemented, not forgotten. There is no single active filament preset — one is
-    //
+    // selected per extruder, and get_config() does not say which extruder the capability runs for —
-    // There is no single active filament preset: one is selected per extruder, and a capability
+    // so guessing (extruder 0, or first override wins) would hand a plugin another extruder's
-    // calls get_config() without saying which extruder it is running for, so we cannot tell which
+    // settings. Filament capabilities read the base config instead. Nothing reaches this today:
-    // of them owns the config. Guessing (first override wins, or extruder 0) would hand a plugin
+    // preset_type_for_capability only names TYPE_FILAMENT once a filament option declares a
-    // another extruder's settings and look like a plugin bug, so it reads the base config instead —
+    // plugin_type, and none does. To lift it, push the extruder onto the plugin call context the
-    // the value it had before presets could override anything.
+    // trampoline already maintains (ScopedPluginAuditContext) and resolve the preset from that. The
-    //
+    // extruder must be optional: whole slicing steps (posSlice, psGCodePostProcess) span every
-    // Nothing reaches this today: preset_type_for_capability only names TYPE_FILAMENT once a
+    // extruder and have no current filament, and this fallback is the honest answer for them.
-    // filament option declares a plugin_type, and none does (the two plugin-backed options are
-    // print and printer ones). Solve it with the first filament-backed capability, by pushing the
-    // extruder onto the plugin call context the trampoline already maintains
-    // (ScopedPluginAuditContext) along with the override text snapshotted off the preset, and
-    // resolving the preset here from that instead of from the bundle. The extruder must be optional:
-    // whole slicing steps (posSlice, psGCodePostProcess) span every extruder and have no current
-    // filament, and this fallback is the honest answer for them.
     case Preset::TYPE_FILAMENT: return nullptr;
 
     default: return nullptr;
@@ -146,8 +139,8 @@ MutationResult PresetPluginConfigService::set_preset_override(CapabilityConfigDo
     const CapabilityConfigId config_id = make_id(id);
     const std::string        version   = running_plugin_version(id.plugin_key);
 
-    // A no-op is a successful unchanged result, not a reason to rewrite the preset: re-saving the
+    // A no-op is a successful unchanged result: re-saving the displayed value must not mark the
-    // displayed value must not be able to mark it dirty.
+    // preset dirty.
     const auto existing = overrides.find(config_id);
     if (existing && existing->cap_config == value && existing->plugin_version == version) {
         result.ok        = true;
@@ -169,7 +162,6 @@ MutationResult PresetPluginConfigService::remove_preset_override(CapabilityConfi
     MutationResult result;
     result.ok      = true;
     result.changed = overrides.erase(make_id(id));
-    // Reads back as the base value now that the override is gone.
     result.effective = get_effective_config(overrides, id);
     return result;
 }
@@ -182,8 +174,7 @@ EffectiveCapabilityConfig active_capability_config(const PluginCapabilityIdentif
     if (const Preset* preset = active_preset_for(preset_type_for_capability(id.type))) {
         std::string error;
         if (!parse_plugin_overrides(plugin_overrides_of(*preset), overrides, error)) {
-            // Text we cannot read is not an override. Say so and resolve against the base config,
+            // Text we cannot read is not an override: log it and resolve against the base config.
-            // which is what the capability ran with before anything was written into the preset.
             BOOST_LOG_TRIVIAL(error) << "Preset \"" << preset->name << "\": " << error;
             overrides = CapabilityConfigDocument();
         }

src/slic3r/plugin/PresetPluginConfig.hpp

@@ -23,7 +23,7 @@ std::string plugin_overrides_of(const Preset& preset);
 bool parse_plugin_overrides(const std::string& raw, CapabilityConfigDocument& document, std::string& error);
 
 // The document as compact JSON text, and "" once it holds no entries. Empty text — rather than a
-// removed option — is what records "cleared here" against an inheriting parent that has overrides.
+// removed option — records "cleared here" against an inheriting parent that has overrides.
 std::string serialize_plugin_overrides(const CapabilityConfigDocument& document);
 
 struct EffectiveCapabilityConfig
@@ -49,12 +49,10 @@ struct MutationResult
 std::string plugin_config_source_to_string(PluginConfigSource source);
 
 // Resolves a capability's effective config as `preset override -> base config -> none`, and mutates
-// the override layer.
+// the override layer. It works on a CapabilityConfigDocument the caller owns, never on a Preset and
-//
+// never on the base config file, which is what keeps the two layers from writing to each other:
-// It works on a CapabilityConfigDocument the caller owns, never on a Preset and never on the base
+// PluginConfigField holds the document and feeds the edited text back through the normal field/dirty
-// config file. That is what keeps the two layers from writing to each other: PluginConfigField holds
+// pipeline, so the preset is written the way every other setting is.
-// the document, and feeds the edited text back through the normal field/dirty pipeline, so the
-// preset is written exactly the way every other setting is.
 class PresetPluginConfigService
 {
 public:
@@ -67,15 +65,11 @@ public:
                                                      const PluginCapabilityIdentifier& id) const;
 };
 
-// The same `preset override -> base config -> none` resolution, against the preset that is active
+// The same resolution against the preset that is active right now, rather than a document the caller
-// right now instead of a document the caller holds: this is what a running capability reads through
+// holds: this is what a running capability reads through the Python config API. Only one preset type
-// the Python config API, so a preset that overrides a capability configures the slice it drives.
+// can reference a given capability type (preset_type_for_capability), so there is exactly one preset
-//
+// to consult. Falls back to the base config for filament capabilities (see active_preset_for) and in
-// Only one preset type can reference a given capability type (preset_type_for_capability), so there
+// a host with no preset bundle (the plugin unit tests).
-// is exactly one preset to consult. Filament capabilities are the exception and are not supported:
-// the active filament preset is per-extruder and a capability does not say which extruder it runs
-// for, so they read the base config. See active_preset_for() for why, and for what it will take to
-// lift that. Base config also in a host with no preset bundle (the plugin unit tests).
 EffectiveCapabilityConfig active_capability_config(const PluginCapabilityIdentifier& id);
 
 } // namespace Slic3r

src/slic3r/plugin/PyPluginTrampoline.hpp

@@ -12,17 +12,14 @@
 #include "PythonJsonUtils.hpp"
 #include "PluginAuditManager.hpp"
 
-// Trampoline variants of pybind11's override macros. Every C++->Python plugin call
+// Trampoline variants of pybind11's override macros. Every C++->Python plugin call crosses a
-// crosses through a trampoline method, so this single boundary is where we (1) log the
+// trampoline method, so this single boundary is where we (1) log the full Python traceback and
-// full Python traceback (to sys.stderr -> session log) and rethrow the exception intact,
+// rethrow the exception intact, and (2) open the plugin's filesystem audit scope for the call.
-// and (2) open the plugin's filesystem audit scope for the duration of the call.
+// We catch ONLY error_already_set (a Python-side raise); other pybind11_fail/runtime_error, such as
-// We catch ONLY error_already_set (a Python-side raise); other pybind11_fail/runtime_error
+// a pure-virtual-missing failure, must keep their own path and are deliberately not caught here.
-// like a pure-virtual-missing failure must keep their own path and are deliberately not
-// caught here.
 
-// Logs (and rethrows) a Python exception from a pybind11 override call, preserving the
+// Shared by the macros below and by trampolines that manage their own audit scope (e.g. the G-code
-// traceback. Internal helper shared by the public macros below and by trampolines that
+// plugin).
-// manage their own audit scope (e.g. the G-code plugin).
 #define ORCA_PY_LOGGED_OVERRIDE_BODY(override_call)                                       \
     try {                                                                                 \
         override_call;                                                                    \
@@ -31,10 +28,9 @@
         throw;                                                                            \
     }
 
-// Opens the plugin's filesystem audit scope for the duration of a C++ -> Python call
+// Opens the plugin's filesystem audit scope for the duration of a C++ -> Python call, and publishes
-// when this trampoline instance carries a non-empty audit plugin key. Also publishes the
+// the calling capability's name so host APIs invoked from Python can tell which capability they are
-// calling capability's name, so host APIs invoked from Python can tell which capability
+// serving. No-op without an audit plugin key. Declares a local `_orca_audit_scope`.
-// they are serving. Declares a local `_orca_audit_scope`.
 #define ORCA_PY_AUDIT_SCOPE(mode)                                                         \
     std::optional<::Slic3r::ScopedPluginAuditContext> _orca_audit_scope;                  \
     if (const std::string& _orca_audit_key = this->audit_plugin_key();                    \
@@ -68,9 +64,8 @@ public:
     }
 
     // Config UI hooks. Available on every capability type, so they live here rather than in
-    // PyPluginInterfaceTrampoline. Audited like any other C++ -> Python call; a Python
+    // PyPluginInterfaceTrampoline. A Python exception is rethrown and the caller decides the
-    // exception is logged with its traceback and rethrown, and the caller (PluginLoader at
+    // fallback.
-    // load time, PluginsDialog when opening the Config tab) decides the fallback.
     bool has_config_ui() const override
     {
         ORCA_PY_OVERRIDE_AUDITED(
@@ -95,15 +90,11 @@ public:
 
     // Hand-rolled rather than PYBIND11_OVERRIDE: the macro casts the Python result to the return
     // type, and nlohmann::json has no pybind caster (config crosses this boundary through the
-    // explicit py_to_json/json_to_py helpers instead). Otherwise identical — same audit scope, and
+    // explicit py_to_json/json_to_py helpers). Otherwise identical — same audit scope, same rethrow.
-    // a Python exception is logged with its traceback and rethrown for the caller to handle.
     //
-    // The hook is optional, and "not implemented" must mean an EMPTY config, never a null or a
+    // The hook is optional, and "not implemented" must mean an EMPTY config: no override, or an
-    // stray scalar landing in cap_config. Two ways to not implement it, both resolved here:
+    // override returning None or any non-object (`def get_default_config(self): pass` is the easy
-    //   - no override at all                     -> the base's empty object
+    // mistake), both fall back to the base's empty object rather than writing `"cap_config": null`.
-    //   - an override that returns None, or any  -> likewise. `def get_default_config(self): pass`
-    //     non-object (a list, a string, a number)   is the easy mistake, and it must not be able to
-    //                                               write `"cap_config": null` to config.json.
     nlohmann::json get_default_config() const override
     {
         ORCA_PY_AUDIT_SCOPE(::Slic3r::PluginAuditManager::AuditMode::Loading);
@@ -127,7 +118,6 @@ public:
         }
     }
 
-    // All plugins may define their own on_load/unload functions.
     void on_load() override
     {
         ORCA_PY_OVERRIDE_AUDITED(
@@ -156,8 +146,6 @@ class PyPluginInterfaceTrampoline : public PyPluginCommonTrampoline<PluginCapabi
 public:
     using PyPluginCommonTrampoline<PluginCapabilityInterface>::PyPluginCommonTrampoline;
 
-    // get_name is implemented in PyPluginCommonTrampoline (PYBIND11_OVERRIDE_PURE).
-
     PluginCapabilityType get_type() const override
     {
         ORCA_PY_OVERRIDE_AUDITED(

src/slic3r/plugin/PythonPluginInterface.hpp

@@ -105,41 +105,33 @@ public:
     // Optional APIs
     virtual PluginCapabilityType get_type() const { return PluginCapabilityType::Unknown; }
 
-    // Every capability is configurable: it always appears in the Plugins dialog's Config
+    // Every capability is configurable and always gets the host's default JSON editor over its
-    // sidebar and always has the host's default JSON editor over its stored config. The only
+    // stored config; this only says whether it supplies its own UI *instead of* that editor.
-    // question a capability answers is whether it supplies its own UI to edit that config
+    // get_config_ui() is called only when it returns true.
-    // *instead of* the JSON editor.
-    //
-    // True when the capability ships a custom configuration UI. get_config_ui() is called
-    // only when this returns true.
     virtual bool has_config_ui() const { return false; }
-    // An HTML snippet for the custom configuration UI. An empty or throwing result is
+    // An HTML snippet for the custom configuration UI. An empty or throwing result is treated as
-    // treated as "no custom UI" and falls back to the default JSON editor.
+    // "no custom UI" and falls back to the default JSON editor.
     virtual std::string get_config_ui() const { return ""; }
 
-    // The config the Config tab's "Restore defaults" action writes back. Optional.
+    // The config the "Restore defaults" action writes back. Not overridden -> an empty object, which
-    //
+    // is right for a capability that keeps its stored config sparse and applies its own defaults on
-    // Not overridden -> an empty object, which is the right answer for a capability that keeps
+    // read. Override it to write an explicit starting config instead (e.g. to seed a form UI with
-    // its stored config sparse and applies its own defaults on read: clearing the overrides
+    // every field present). The host neither invents nor validates this value, so a throwing override
-    // *is* restoring the defaults, and it keeps a later release free to change them.
+    // leaves the stored config untouched.
-    // Override it to write an explicit starting config instead (e.g. to seed a form UI with
-    // every field present). The host neither invents nor validates this value; it only stores
-    // whatever comes back, so a throwing override leaves the stored config untouched.
     virtual nlohmann::json get_default_config() const { return nlohmann::json::object(); }
 
     virtual void on_load() {}
     virtual void on_unload() {}
 
-    // C++-only audit identity (never exposed to Python). Set by PluginLoader after
+    // C++-only audit identity (never exposed to Python), set by PluginLoader after plugin capture so
-    // plugin capture so trampoline calls can scope filesystem enforcement to this
+    // trampoline calls can scope filesystem enforcement to this plugin. PluginDescriptor::plugin_key.
-    // plugin. This is PluginDescriptor::plugin_key, the canonical runtime id.
     void set_audit_plugin_key(std::string key) { m_audit_plugin_key = std::move(key); }
     const std::string& audit_plugin_key() const { return m_audit_plugin_key; }
 
-    // The cached get_name() captured at load, paired with the audit plugin key to identify
+    // get_name() cached at load, paired with the audit plugin key to identify which capability a
-    // which capability a trampoline call belongs to. Cached rather than read live: get_name()
+    // trampoline call belongs to. Cached rather than read live: get_name() is itself a trampoline
-    // is itself a trampoline call, so calling it from inside a trampoline would recurse.
+    // call, so calling it from inside a trampoline would recurse. Empty until PluginLoader
-    // Empty until PluginLoader materializes the capability.
+    // materializes the capability.
     void set_audit_capability_name(std::string name) { m_audit_capability_name = std::move(name); }
     const std::string& audit_capability_name() const { return m_audit_capability_name; }
 

tests/slic3rutils/test_plugin_capabilities_in_use.cpp

@@ -12,8 +12,7 @@ using namespace Slic3r;
 
 namespace {
 
-// A print preset carrying a "plugins" manifest and the one plugin-backed print option
+// A print preset carrying a "plugins" manifest and the one plugin-backed print option.
-// (slicing_pipeline_plugin, a coStrings vector).
 Preset make_print_preset(const std::vector<std::string>& manifest, const std::vector<std::string>& pipeline)
 {
     Preset preset(Preset::TYPE_PRINT, "test-print");
@@ -76,16 +75,15 @@ TEST_CASE("referenced_capabilities skips malformed manifest entries", "[PluginRe
 
 TEST_CASE("preset_type_for_capability names the preset type whose options reference the capability", "[PluginResolver]")
 {
-    // Read out of the ConfigDef: slicing_pipeline_plugin is a print option, printer_agent a printer
+    // Read out of the ConfigDef: declaring a plugin_type on an option is what puts its capability
-    // one. Declaring a plugin_type on an option is what puts its capability type on this map.
+    // type on this map.
     CHECK(preset_type_for_capability(PluginCapabilityType::SlicingPipeline) == Preset::TYPE_PRINT);
     CHECK(preset_type_for_capability(PluginCapabilityType::PrinterConnection) == Preset::TYPE_PRINTER);
 }
 
 TEST_CASE("preset_type_for_capability leaves capability types no option accepts unowned", "[PluginResolver]")
 {
-    // Nothing can reference these from a preset, so no preset can override them either: they read
+    // No option accepts them, so no preset can override them: they read config.json alone.
-    // their config from the base config.json alone.
     CHECK(preset_type_for_capability(PluginCapabilityType::Automation) == Preset::TYPE_INVALID);
     CHECK(preset_type_for_capability(PluginCapabilityType::Unknown) == Preset::TYPE_INVALID);
 }

tests/slic3rutils/test_plugin_capability_config.cpp

@@ -23,8 +23,8 @@ namespace {
 
 void ensure_python_initialized()
 {
-    // Same rationale as test_plugin_host_api.cpp: `orca` is an embedded module compiled into this
+    // As in test_plugin_host_api.cpp: `orca` is compiled into this binary, so a bare interpreter is
-    // binary, so a bare interpreter is enough and does not need the bundled Python home.
+    // enough and does not need the bundled Python home.
     if (!Py_IsInitialized()) {
         static py::scoped_interpreter interpreter;
         (void) interpreter;
@@ -38,16 +38,15 @@ py::module_ import_orca_module()
     return py::module_::import("orca");
 }
 
-// Builds a Python capability from `body` and materializes it the way PluginLoader does: the audit
+// Builds a Python capability the way PluginLoader does: the audit identity is stamped on by the
-// identity is stamped on by the host, never supplied by the plugin, and it is what scopes every
+// host, never supplied by the plugin, and it scopes every config call to this one capability.
-// config call to this one capability.
 py::object make_capability(const std::string& class_name,
                            const std::string& body,
                            const std::string& plugin_key,
                            const std::string& capability_name)
 {
-    // Import first: it is what brings the interpreter up, and constructing any py:: object
+    // Import first: it brings the interpreter up, and any py:: object built before it would touch a
-    // beforehand would touch a Python that does not exist yet.
+    // Python that does not exist yet.
     py::module_ orca = import_orca_module();
 
     py::dict globals;
@@ -69,12 +68,10 @@ std::shared_ptr<PluginCapabilityInterface> as_interface(const py::object& instan
     return instance.cast<std::shared_ptr<PluginCapabilityInterface>>();
 }
 
-// The config the Python API actually writes to: capability_save_config persists through the
+// The Python API writes through the PluginManager singleton, so that is where assertions read from.
-// PluginManager singleton, so that is where the assertions read from.
 PluginConfig& host_config() { return PluginManager::instance().get_config(); }
 
-// The Python config API speaks JSON text, not dicts: get_config()/get_default_config() hand back a
+// The Python config API speaks JSON text, not dicts; these helpers keep the tests in terms of values.
-// JSON string and save_config() takes one. These helpers keep the tests written in terms of values.
 json py_get_config(const py::object& cap) { return json::parse(cap.attr("get_config")().cast<std::string>()); }
 
 bool py_save_config(const py::object& cap, const json& value) { return cap.attr("save_config")(value.dump()).cast<bool>(); }
@@ -87,8 +84,8 @@ TEST_CASE("Capability config API is exposed on every Python capability", "[Plugi
     REQUIRE(py::hasattr(orca, "PythonPluginBase"));
 
     py::object base = orca.attr("PythonPluginBase");
-    // Host-provided (the capability calls these). Every capability has a config, so these are
+    // Host-provided: every capability has a config, so there is no hook to opt out of being
-    // always available — there is no hook to opt in or out of being configurable.
+    // configurable.
     CHECK(py::hasattr(base, "get_config"));
     CHECK(py::hasattr(base, "save_config"));
     CHECK(py::hasattr(base, "get_config_version"));
@@ -97,8 +94,8 @@ TEST_CASE("Capability config API is exposed on every Python capability", "[Plugi
     CHECK(py::hasattr(base, "get_config_ui"));
     CHECK(py::hasattr(base, "get_default_config"));
 
-    // Config is reached through the capability, never as a free orca.config.* function, so a
+    // Config is reached only through the capability, never as a free orca.config.* function, so a
-    // capability cannot name — and therefore cannot touch — a config that is not its own.
+    // capability cannot name — and cannot touch — a config that is not its own.
     CHECK_FALSE(py::hasattr(orca, "config"));
 }
 
@@ -109,8 +106,8 @@ TEST_CASE("get_config returns only cap_config and save_config persists it", "[Pl
 
     py::object cap = make_capability("RoundTripCap", "    def get_name(self): return 'cap_a'\n", "plugin_a", "cap_a");
 
-    // Nothing stored yet: the JSON text of an empty object, not None, so a plugin can json.loads()
+    // Nothing stored yet: the JSON text of an empty object, not None, so a plugin can json.loads() it
-    // and index it unconditionally.
+    // unconditionally.
     py::object initial = cap.attr("get_config")();
     REQUIRE(py::isinstance<py::str>(initial));
     CHECK(json::parse(initial.cast<std::string>()) == json::object());
@@ -118,12 +115,11 @@ TEST_CASE("get_config returns only cap_config and save_config persists it", "[Pl
 
     REQUIRE(py_save_config(cap, json{{"speed", 5}, {"name", "fast"}}));
 
-    // Persisted through PluginConfig, under this capability's identity only.
     const BaseConfig stored = host_config().get_config("plugin_a", "cap_a");
     REQUIRE_FALSE(stored.empty());
     CHECK(stored.config == json{{"speed", 5}, {"name", "fast"}});
 
-    // And read back through Python as exactly cap_config — no host metadata.
+    // Python reads back exactly cap_config — no host metadata.
     const json reloaded = py_get_config(cap);
     CHECK(reloaded.size() == 2);
     CHECK(reloaded.contains("speed"));
@@ -142,8 +138,7 @@ TEST_CASE("save_config rejects a string that is not valid JSON", "[PluginConfig]
 
     REQUIRE(py_save_config(cap, json{{"keep", "me"}}));
 
-    // The binding parses the string it is handed. Unparseable text is refused, and refusing it must
+    // Refusing unparseable text must leave the previously stored config alone.
-    // leave the previously stored config alone rather than clobbering it with nothing.
     CHECK_FALSE(cap.attr("save_config")("{not json").cast<bool>());
     CHECK(host_config().get_config("plugin_a", "cap_a").config == json{{"keep", "me"}});
 }
@@ -154,8 +149,8 @@ TEST_CASE("Saving one capability's config does not touch another's", "[PluginCon
     host_config().load();
 
     const std::string body = "    def get_name(self): return 'cap'\n";
-    // Same capability name under two different plugins, plus a second capability of plugin_a:
+    // Same capability name under two plugins, plus a second capability of plugin_a: each addresses
-    // each addresses only the entry matching its own stamped identity.
+    // only the entry matching its own stamped identity.
     py::object a_cap1 = make_capability("IsoCapA1", body, "plugin_a", "cap_a");
     py::object a_cap2 = make_capability("IsoCapA2", body, "plugin_a", "cap_b");
     py::object b_cap1 = make_capability("IsoCapB1", body, "plugin_b", "cap_a");
@@ -170,7 +165,6 @@ TEST_CASE("Saving one capability's config does not touch another's", "[PluginCon
     CHECK(host_config().get_config("plugin_a", "cap_b").config == json{{"value", 2}});
     CHECK(host_config().get_config("plugin_b", "cap_a").config == json{{"value", 3}});
 
-    // Each capability still reads back its own value.
     CHECK(py_get_config(a_cap2).at("value") == 2);
     CHECK(py_get_config(b_cap1).at("value") == 3);
 }
@@ -180,8 +174,8 @@ TEST_CASE("Config API refuses a capability the host never materialized", "[Plugi
     ScopedDataDir data_dir_guard("plugin-config-py-unowned");
     host_config().load();
 
-    // No audit identity: the instance was never loaded by the host, so it has no config to address.
+    // No audit identity: never loaded by the host, so it has no config to address. Refused rather
-    // Refused rather than served from, or written to, some arbitrary entry.
+    // than served from, or written to, some arbitrary entry.
     py::object orphan = make_capability("OrphanCap", "    def get_name(self): return 'cap'\n", "", "");
 
     CHECK_THROWS(orphan.attr("get_config")());
@@ -208,9 +202,8 @@ TEST_CASE("A capability that omits the config UI hooks gets the default editor",
     ScopedDataDir data_dir_guard("plugin-config-py-bare");
     host_config().load();
 
-    // Both hooks are optional and only choose the editor. A capability that overrides neither is
+    // Both hooks are optional and only choose the editor: a capability that overrides neither is
-    // still configurable — it just gets the host's JSON editor — so it stays in the Config sidebar
+    // still configurable, it just gets the host's JSON editor. There is no way to opt out.
-    // and its config API keeps working. There is no way for a capability to opt out of having one.
     py::object bare = make_capability("BareCap", "    def get_name(self): return 'cap_a'\n", "plugin_a", "cap_a");
 
     auto iface = as_interface(bare);
@@ -226,8 +219,8 @@ TEST_CASE("get_default_config supplies the value Restore defaults writes back",
 {
     SECTION("not overridden -> an empty config")
     {
-        // Which is already "restore defaults" for a capability that keeps its stored config sparse
+        // Already "restore defaults" for a capability that keeps its stored config sparse and applies
-        // and applies its own defaults on read: clearing the overrides restores them.
+        // its own defaults on read.
         py::object bare = make_capability("NoDefaultsCap", "    def get_name(self): return 'cap_a'\n", "plugin_a", "cap_a");
 
         auto iface = as_interface(bare);
@@ -251,8 +244,8 @@ TEST_CASE("get_default_config supplies the value Restore defaults writes back",
 
     SECTION("overridden but returns None -> an empty config, never a null")
     {
-        // `def get_default_config(self): pass` is the easy mistake. It must not be able to store
+        // `def get_default_config(self): pass` is the easy mistake, and it must not store
-        // "cap_config": null — an unimplemented hook means an empty config, however it is spelled.
+        // "cap_config": null.
         py::object cap = make_capability("NoneDefaultsCap",
                                          "    def get_name(self): return 'cap_a'\n"
                                          "    def get_default_config(self): pass\n",
@@ -298,7 +291,6 @@ TEST_CASE("Restoring defaults overwrites only the target capability", "[PluginCo
     REQUIRE(host_config().store_capability_config("plugin_a", "cap_a", iface->get_default_config()));
 
     CHECK(host_config().get_config("plugin_a", "cap_a").config == json{{"speed", 1}});
-    // The same capability name under another plugin keeps its edited value.
     CHECK(host_config().get_config("plugin_b", "cap_a").config == json{{"speed", 99}});
 }
 
@@ -333,19 +325,16 @@ TEST_CASE("A raising config UI hook surfaces as an exception the host can catch"
     auto iface = as_interface(cap);
     REQUIRE(iface);
 
-    // The trampoline logs the traceback and rethrows; callers (PluginLoader when caching the flag,
+    // The trampoline rethrows; callers catch it and fall back to the default JSON editor.
-    // PluginsDialog when opening the Config tab) catch it and fall back to the default JSON editor
-    // instead of crashing.
     CHECK_THROWS_AS(iface->get_config_ui(), py::error_already_set);
 
-    // Catching it leaves the interpreter usable — the host is still able to talk to the capability.
+    // Catching it leaves the interpreter usable.
     CHECK(iface->get_name() == "cap_a");
 }
 
 TEST_CASE("A config UI hook returning the wrong type does not crash the host", "[PluginConfig][Python]")
 {
-    // has_config_ui() is plugin-authored, so it can return anything. Whatever pybind makes of a
+    // has_config_ui() is plugin-authored, so it can return anything; the host must survive the call.
-    // non-bool, the host must survive the call: it either converts or throws, never crashes.
     py::object cap = make_capability("BadTypeCap",
                                      "    def get_name(self): return 'cap_a'\n"
                                      "    def has_config_ui(self): return 'not a bool'\n",
@@ -354,15 +343,15 @@ TEST_CASE("A config UI hook returning the wrong type does not crash the host", "
     auto iface = as_interface(cap);
     REQUIRE(iface);
 
-    // Deliberately not REQUIRE_THROWS: pybind may coerce the value or reject it, and both are
+    // Deliberately not REQUIRE_THROWS: pybind may coerce or reject the value, and both are fine.
-    // acceptable. What must hold is that the call is survivable — a throw is what PluginLoader's
+    // What must hold is that the call is survivable — PluginLoader's guard turns a throw into
-    // guard turns into "no custom UI".
+    // "no custom UI".
     try {
         (void) iface->has_config_ui();
     } catch (const std::exception&) {
     }
 
-    // The capability is still usable afterwards: the bad hook cost it nothing but its own answer.
+    // The capability is still usable afterwards.
     CHECK(iface->get_name() == "cap_a");
     CHECK(iface->get_config_ui().empty());
 }

tests/slic3rutils/test_plugin_config.cpp

@@ -65,8 +65,7 @@ TEST_CASE("PluginConfig updates only the target capability's cap_config", "[Plug
     ScopedDataDir data_dir_guard("plugin-config-isolation");
 
     PluginConfig config;
-    // Two capabilities in one plugin, plus a same-named capability in a different plugin: the
+    // The identity is the (plugin_key, capability) pair, so all three below are separate records.
-    // identity is the (plugin_key, capability) pair, so all three are separate records.
     REQUIRE(config.store_capability_config("plugin_a", "cap_a", json{{"value", 1}}));
     REQUIRE(config.store_capability_config("plugin_a", "cap_b", json{{"value", 2}}));
     REQUIRE(config.store_capability_config("plugin_b", "cap_a", json{{"value", 3}}));
@@ -116,9 +115,8 @@ TEST_CASE("PluginConfig keeps a capability's config after its plugin goes away",
         REQUIRE(config.store_capability_config("plugin_a", "cap_a", json{{"token", "keep me"}}));
     }
 
-    // Nothing here installs, uninstalls or unsubscribes a plugin: config.json is deliberately not
+    // config.json is deliberately not keyed to installed plugins: a record outlives its plugin and is
-    // keyed to installed plugins, so a record outlives the plugin and is still there when the user
+    // still there on reinstall. Asserts no cleanup path silently drops it.
-    // reinstalls or resubscribes. This asserts no cleanup path silently drops it.
     PluginConfig after_removal;
     after_removal.load();
     CHECK(after_removal.get_config("plugin_a", "cap_a").config == json{{"token", "keep me"}});