Ver código fonte

Refactor region def with helpers, refine docs

Extract the inline cursor-walk in handleRegionCmd into file-local
helpers (skipSpaces, rtrimSpaces, takeToken, splitNameJump,
processRegionDefSegment), grouped immediately above the consumer.
Behavior is identical; addresses PR #2540 review feedback on
readability.

Tighten the region def docs: collapse five Note callouts into three
grouped paragraphs (Behavior / Existing regions / Limits), add a
case-sensitivity caveat plus an error example, note the cursor reset
between split commands, and use generic placeholder names.
agessaman 2 meses atrás
pai
commit
1296aa7792
2 arquivos alterados com 77 adições e 73 exclusões
  1. 17 16
      docs/cli_commands.md
  2. 60 57
      src/helpers/CommonCLI.cpp

+ 17 - 16
docs/cli_commands.md

@@ -762,35 +762,36 @@ This document provides an overview of CLI commands that can be sent to MeshCore
 
 **Parameters (tokens):** Space-separated. A logical **cursor** starts at the wildcard `*`.
 
-- **`name`** — Create `name` as a child of the current cursor (same as `region put name` with that parent). Cursor moves to `name`.
-- **`name|jump`** or **`name,jump`** — Create `name` as a child of the current cursor, then move the cursor to `jump` (must already exist: created earlier in this command or already on the node). `jump` is **not** the parent of `name`; use this to pop back up and start another branch.
+- **`name`** — Create `name` as a child of the current cursor (equivalent to `region put name` with the cursor as parent). Cursor moves to `name`.
+- **`name|jump`** *(or `name,jump`)* — Create `name` as a child of the current cursor, then move the cursor to `jump` (must already exist on the node, or have been created earlier in this command). `jump` is **not** the parent of `name`; use this form to pop back up and start another branch.
 
-**Note:** Same flood defaults as `region put` (flood allowed on each created region).
+**Behavior:** Each created region defaults to flood-allowed (same as `region put`). The reply is the resulting region tree (same format as bare `region`); review it before running `region save` to persist. On error, the reply is `Err - ...` and any regions placed before the failure remain on the node, just like a partial chain of `region put`.
 
-**Note:** Does **not** persist to flash. The reply is the region tree (same format as bare `region`) so you can review before **`region save`**.
+**Existing regions:** `region def` does not clear the existing tree — if a name already exists, its parent is updated to the current cursor; otherwise a new region is created. To start from scratch, `region remove` the unwanted regions first.
 
-**Note:** On error, the reply is a short `Err - ...` message; regions placed before the failure remain (same as a partial chain of `region put`).
+**Limits:** Repeater serial accepts one line up to **160 characters**. For larger trees, split across multiple `region def` commands; the cursor resets to `*` between commands, so lead the next command with `child|ancestor` to reposition. Each token splits at most once on `|` — `region def a|b|c|d` is not a flat-list shorthand; see the flat-list example below.
 
-**Note:** Repeater serial accepts one line up to **160 characters** total; split very large trees across multiple `region def` commands.
-
-**Note:** `|` only splits once per token. `region def a|b|c|d` is **not** a flat-list shorthand — use `region def a|* b|* c|* d|*` for multiple children of `*`.
-
-**Example — linear chain:**
+**Example — linear chain** (each token becomes a child of the previous):
 ```
-region def west pnw wa w-wa sea
+region def a b c d e
 region save
 ```
 
-**Example — branched tree** (equivalent to `region put west` … `region put sw-wa wa`):
+**Example — branched tree** (equivalent to `region put a`, `region put b a`, `region put c b`, `region put d c`, `region put e b`, `region put f e`):
 ```
-region def west pnw or pdx|pnw wa sw-wa
+region def a b c d|b e f
 region save
 ```
-Same with comma as jump delimiter: `region def west pnw or pdx,pnw wa sw-wa`
 
-**Example — flat list** (each region child of `*`):
+**Example — error and partial state:**
+```
+region def a b c|nope d
+```
+The reply is `Err - unknown jump: nope`. `a`, `b`, and `c` were placed before the failure; `d` was not. Run `region` to inspect, then re-run with a corrected jump or repair with `region remove` / `region put`.
+
+**Example — flat list** (each region a child of `*`). Use `|*` after each token to pop the cursor back to the root before the next token:
 ```
-region def west|* pnw|* or|* pdx|* wa|* sw-wa
+region def a|* b|* c|* d|* e|* f
 region save
 ```
 

+ 60 - 57
src/helpers/CommonCLI.cpp

@@ -895,68 +895,71 @@ void CommonCLI::handleGetCmd(uint32_t sender_timestamp, char* command, char* rep
   }
 }
 
+static char* skipSpaces(char* s) {
+  while (*s == ' ') s++;
+  return s;
+}
+
+static void rtrimSpaces(char* s) {
+  char* e = s + strlen(s);
+  while (e > s && e[-1] == ' ') *--e = '\0';
+}
+
+static char* takeToken(char** cursor) {
+  char* p = skipSpaces(*cursor);
+  if (*p == '\0') { *cursor = p; return nullptr; }
+  char* tok = p;
+  while (*p && *p != ' ') p++;
+  if (*p) *p++ = '\0';
+  *cursor = p;
+  return tok;
+}
+
+static char* splitNameJump(char* tok) {
+  for (char* q = tok; *q; q++) {
+    if (*q == '|' || *q == ',') {
+      *q = '\0';
+      char* jump = skipSpaces(q + 1);
+      rtrimSpaces(jump);
+      return jump;
+    }
+  }
+  return nullptr;
+}
+
+static bool processRegionDefSegment(RegionMap* map, char* tok, RegionEntry** cursor, char* reply) {
+  char* jump = splitNameJump(tok);
+  char* name = skipSpaces(tok);
+  if (*name == '\0') { snprintf(reply, 160, "Err - empty name"); return false; }
+  if (jump && *jump == '\0') { snprintf(reply, 160, "Err - empty jump"); return false; }
+
+  RegionEntry* r = map->putRegion(name, (*cursor)->id);
+  if (r == NULL) { snprintf(reply, 160, "Err - put failed: %s", name); return false; }
+  r->flags = 0;
+
+  if (jump) {
+    RegionEntry* j = map->findByNamePrefix(jump);
+    if (j == NULL) { snprintf(reply, 160, "Err - unknown jump: %s", jump); return false; }
+    *cursor = j;
+  } else {
+    *cursor = r;
+  }
+  return true;
+}
+
 void CommonCLI::handleRegionCmd(char* command, char* reply) {
   reply[0] = 0;
 
-  // `region def ...` — cursor-walk over space-separated tokens (must run before
-  // parseTextParts, which only keeps 4 segments and mutates the buffer).
-  char* cmd = command;
-  while (*cmd == ' ') cmd++;
+  // `region def`: must run before parseTextParts mutates the buffer
+  char* cmd = skipSpaces(command);
   if (strncmp(cmd, "region def", 10) == 0 && (cmd[10] == ' ' || cmd[10] == '\0')) {
-    char* payload = cmd + 10;
-    while (*payload == ' ') payload++;
-    if (*payload == '\0') {
-      snprintf(reply, 160, "Err - empty def");
-      return;
-    }
-    RegionEntry* cursor = &_region_map->getWildcard();
-    char* p = payload;
-    while (*p) {
-      while (*p == ' ') p++;
-      if (*p == '\0') break;
-      char* tok = p;
-      while (*p && *p != ' ') p++;
-      if (*p) *p++ = '\0';
+    char* payload = skipSpaces(cmd + 10);
+    rtrimSpaces(payload);
+    if (*payload == '\0') { snprintf(reply, 160, "Err - empty def"); return; }
 
-      char* jump = nullptr;
-      for (char* q = tok; *q; q++) {
-        if (*q == '|' || *q == ',') {
-          *q = '\0';
-          jump = q + 1;
-          break;
-        }
-      }
-      char* name = tok;
-      while (*name == ' ') name++;
-      if (jump) {
-        while (*jump == ' ') jump++;
-        char* je = jump + strlen(jump);
-        while (je > jump && je[-1] == ' ') *--je = '\0';
-      }
-      if (*name == '\0') {
-        snprintf(reply, 160, "Err - empty name");
-        return;
-      }
-      if (jump && *jump == '\0') {
-        snprintf(reply, 160, "Err - empty jump");
-        return;
-      }
-      auto r = _region_map->putRegion(name, cursor->id);
-      if (r == NULL) {
-        snprintf(reply, 160, "Err - put failed: %s", name);
-        return;
-      }
-      r->flags = 0;
-      if (jump) {
-        auto j = _region_map->findByNamePrefix(jump);
-        if (j == NULL) {
-          snprintf(reply, 160, "Err - unknown jump: %s", jump);
-          return;
-        }
-        cursor = j;
-      } else {
-        cursor = r;
-      }
+    RegionEntry* cursor = &_region_map->getWildcard();
+    for (char* tok; (tok = takeToken(&payload)) != nullptr; ) {
+      if (!processRegionDefSegment(_region_map, tok, &cursor, reply)) return;
     }
     _region_map->exportTo(reply, 160);
     return;