Update the spec for Action IDs

doc/specs/#6899 - Action IDs/#6899 - Action IDs.md

@@ -23,7 +23,7 @@ contexts without needing to replicate an entire json blob.
 
 This spec was largely inspired by the following diagram from @DHowett:
 
-![figure 1](data-mockup.png)
+![figure 1](data-mockup-002.png)
 
 The goal is to introduce an `id` parameter by which actions could be uniquely
 referred to. If we'd ever like to use an action outside the list of `actions`, we
@@ -36,38 +36,54 @@ Discussion with the team lead to the understanding that the name `actions` would
 be even better, as a way of making the meaning of the "list of actions" more
 obvious.
 
-When we're parsing `actions`, we'll make three passes:
-* The first pass will scan the list for objects with an `id` property. We'll
+We will then restructure `defaults.json`, and also users' settings files (via `fixUpUserSettings`), in the following manner:
+* Instead of each `command` json block containing both the `action` (along with additional arguments) and the `keys`, these will now be split up -
+    * There will now be one json block for just the `command`/`action`, which will also contain the `id`. These json blocks will be in their own list called `actions`.
+    * There will be another json block for the `keys`, which will refer to the action to be invoked by `id`. These json blocks will be in their own list called `keybindings`.
+
+For example, let's take a look at the `split pane right` action in `defaults.json` as we currently have it:
+
+`"actions": [..., { "command": { "action": "splitPane", "split": "right" }, "keys": "alt+shift+plus" }, ...]`
+
+This will now become:
+
+`"actions": [..., { "command": { "action": "splitPane", "split": "right" }, "id": "Terminal.SplitPaneRight" }, ...]`
+
+`"keybindings": [..., { "keys": "alt+shift+plus", "id": "Terminal.SplitPaneRight" }, ...]`
+
+Here is how we will parse settings file and construct the relevant settings model objects:
+* We will first scan the `actions` list. We'll
   attempt to parse those entries into `ActionAndArgs` which we'll store in the
-  global `id->ActionAndArgs` map. If any entry doesn't have an `id` set, we'll
-  skip it in this phase. If an entry doesn't have a `command` set, we'll ignore
-  it in this pass.
-* The second pass will scan for _keybindings_. Any entries with `keys` set will
-  create a `KeyChord->ActionAndArgs` entry in the keybindings map. If the entry
-  has an `id` set, then we'll simply re-use the action we've already parsed for
-  the `id`, from the action map. If there isn't an `id`, then we'll parse the
-  action manually at this time. Entries without a `keys` set will be ignored in
-  this pass.
-* The final pass will be to generate _commands_. Similar to the keybindings
-  pass, we'll attempt to lookup actions for entries with an `id` set. If there
-  isn't an `id`, then we'll parse the action manually at this time. We'll then
-  get the name for the entry, either from the `name` property if it's set, or
-  the action's `GenerateName` method.
+  global `id->ActionAndArgs` map. All actions defined in `defaults.json` must have an `id` specified, and all actions provided by fragments must also have `id`s. Any actions from the defaults or fragments that do not provide `id`s will be ignored. As for user-specified commands, if no `id` is set, we will auto-generate one for that command based on the action and any additional arguments. For example, the `split pane right` command above might result in an autogenerated id `User.SplitPaneRight`.
+    * Note: this step is also where we will generate _commands_. We will use the name provided in the entry if it's set or the action's `GenerateName` method.
+* Next we will scan the `keybindings` list. These entries will
+  create a `KeyChord->ActionAndArgs` entry in the keybindings map. Since these objects should all contain an `id`, we will simply use the `id->ActionAndArgs` map we created in the previous step. Any object with `keys` set but no `id` will be ignored.
 
-For a visual representation, let's assume the user has the following in their
-`actions`:
+For a visual representation:
 
-![figure 2](data-mockup-actions.png)
+![figure 2](data-mockup-actions-ids-keys-maps.png)
 
-We'll first parse the `actions` to generate the mapping of `id`->`Actions`:
+### Nested actions
 
-![figure 3](data-mockup-actions-and-ids.png)
+We allow certain actions that take a form like this:
 
-Then, we'll parse the `actions` to generate the mapping of keys to actions, with
-some actions already being defined in the map of `id`->`Actions`:
+```
+        {
+            // Select color scheme...
+            "name": { "key": "SetColorSchemeParentCommandName" },
+            "commands": [
+                {
+                    "iterateOn": "schemes",
+                    "name": "${scheme.name}",
+                    "command": { "action": "setColorScheme", "colorScheme": "${scheme.name}" }
+                }
+            ]
+        }
+```
 
-![figure 4](data-mockup-actions-and-ids-and-keys.png)
+For this case, having an `id` on the top level could potentially make sense when it comes to using that `id` in a menu, but not in the case of using that `id` for a keybinding. For the initial implementation, we will not support an `id` for these types of actions, which leaves us open to revisiting this in the future.
 
+### Layering
 
 When layering `actions`, if a later settings file contains an action with the
 same `id`, it will replace the current value. In this way, users can redefine
@@ -87,6 +103,9 @@ As we add additional menus to the Terminal, like the customization for the new
 tab dropdown, or the tab context menu, or the `TermControl` context menu, they
 could all refer to these actions by `id`, rather than duplicating the same json.
 
+As for fragments, all actions in fragments _must_ have an `id`. If a fragment provides an action without an `id`, or provides an `id` that clashes with one of the actions in `defaults.json`, that action will be ignored.
+
+> 👉 NOTE: This will mean that actions will now need an `OriginTag`, similar to profiles and color schemes
 
 ### Existing Scenarios