Repository navigation
Explorer: Rename Class Category + Rename Dictionary (R6) - #402
Merged
Merged
Conversation
Rename class category (the R6 catalog gap) and rename dictionary, both as non-committing server metadata operations via the GemStone Explorer — no RB engine clone, nothing recompiled or committed. Rename class category: - queries/renameClassCategory.ts reassigns the exact category and its dash-segmented subtree via Class>>category:. Category comparison uses Unicode-safe `asSymbol ==` (3.6.x raises error 2718 on String = of a wide category), so an ASCII rename is safe even when the dictionary contains a wide-named category. - Handler renames a single node only: prompt is seeded with the last segment, dashes are rejected, and the full path is rebuilt from the unchanged parent — a rename can't graft new subtree levels or leave a trailing-'-' empty node. - Version-aware non-ASCII guard: pre-3.7 stones can't compile a doit whose source carries a wide string literal (error 1001, ComStrmSetCursor), so on <3.7 a non-ASCII old/new name is refused with a clear message instead of a compiler crash; 3.7+ (the server-UTF8 boundary) proceeds normally. - Pencil-only: the right-click menu entry is removed, matching PR #397. Rename dictionary: - queries/renameDictionary.ts swaps the self-referential association in place (keeps symbol-list position), guards system dictionaries and name collisions. Tests: query + handler unit tests (single-node restriction, parent rebuild, no-dash and non-ASCII validation, version gating) and live integration tests for both category and dictionary rename. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…DictR6 # Conflicts: # client/src/__tests__/explorerQueries.integration.test.ts # client/src/gemstoneExplorer.ts
…on-absent Follow-up to the dict-lookup consolidation now on main: swap renameClassCategory and renameDictionary off the hand-rolled `symbolList at:/objectNamed:` ternary to the shared dictLookupExpr helper, so the noHandRolledDictLookup guard test (merged from main) passes on this branch too. The swap exposed a regression: renameDictionary has an out-of-range-index -> 'Dictionary not found' test, but dictLookupExpr's bare numeric `at:` raised on a bad index instead of returning nil. Make the helper uniformly nil-on-absent (`at: N ifAbsent: [nil]`), matching its name branch, so callers' `ifNil:` guards work for both forms. The output is still a substring of `symbolList at: N`, so the existing swept-query assertions are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MED-1: after a dictionary rename, sweep open editor tabs whose URIs still name the
old dictionary -- close the clean ones, warn about dirty ones (new
closeStaleTabsForRenamedDictionary, reusing listOpenGemstoneTabs/parseUri).
MED-2: renameClassCategory now parses `renamed: <n>` -- an unrecognised payload is an
error, and 0 moved while the client expected classes surfaces a warning, not a false
success.
MED-3: always run the category-rename query instead of gating on the possibly-stale
classCategoryEntries cache.
LOW-1: `cat size >= prefix size` so a category named exactly `<old>-` moves with its
subtree instead of being left behind.
LOW-2: count classes whose category can't be read and report `renamed: N skipped: M`;
the handler warns when any were skipped.
LOW-3: add the 1_modification@0 right-click entry for renameClassCategory, matching
renameDictionary.
LOW-4: replace a tautological `expect(out).toBe('ok')` with an assertion on the
emitted code.
LOW-5: renaming the selected dictionary retains the class/method selection via
refreshRetainingSelection instead of selectDict.
LOW-6: document why classLookupExpr's index branch intentionally doesn't guard, unlike
the nil-safe dictLookupExpr.
The emitted Smalltalk is kept ASCII-only (rationale moved to TS comments) so the doit
doesn't trip the 3.6.x compiler (ComStrmSetCursor, err 1001). Full suite green:
client 5267, server 322, mcp 92.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MatiasFernandez
approved these changes
Aug 11, 2026
MatiasFernandez
left a comment
MatiasFernandez
left a comment
Collaborator
There was a problem hiding this comment.
Left some comments worth reviewing
ericwinger
commented
Aug 11, 2026
ericwinger
left a comment
ericwinger
left a comment
Member
Author
There was a problem hiding this comment.
Thanks for the thorough pass. All nine comments addressed — replies inline on each. Summary: guarded the category write (#1) and widened the dictionary type guard (#3); kept dictLookupExpr nil-on-absent for user-friendliness and reconciled both helper docs (#2); comment/manifest polish (#4, #9); and added the missing coverage — the tab-sweep suite (#5), the overlay-carry + classCategory remap case (#6), a live-stone contents-reachability assertion (#7), and split the cancelled/unchanged test (#8).
- renameClassCategory: guard the `category:` write in `on: Error do:` and count failures into `skipped`, so one un-reassignable class degrades to `renamed: N skipped: M` instead of unwinding the batch and half-renaming - renameDictionary: reject a non-SymbolDictionary target with a tidy refusal instead of a raw DNU; name the `name:`/`removeKey:` selectors in the comment - util: keep dictLookupExpr numeric branch nil-on-absent (avoids a raw index-out-of-range walkback on a stale tree) and reconcile the dictLookupExpr / classLookupExpr docs so they explain the deliberate divergence - tests: focused closeStaleTabsForRenamedDictionary sweep suite; overlay-carry + state.classCategory remap case; live-stone contents-reachability assertion for renameDictionary; split the cancelled/unchanged category test - package.json: pin removeDictionary inline@1 after the rename pencil Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ericwinger
added a commit
that referenced
this pull request
Aug 11, 2026
Brings in PR #402 (R6 rename category/dictionary), #404 (dedup ownInstVarsOf:/allInstVarsOf: onto GsRefactoringEnvironment) and #405 (Unicode7 doit-literal fixes), plus the npm supply-chain hardening. One conflict: resources/refactoring/manifest.gs, on the GsRefactoringEnvironment method count -- this branch had 22 (it adds one environment method), main had 23 (#404 added two). NEITHER side is correct for the merged source, which defines 24; taking either would fail the loader's post-load method-count check. Resolved by regenerating all payloads from the merged gs-src via build-refactoring.sh rather than hand-picking a number, so engine.gs / engine-tests.gs / manifest.gs are consistent with the source they are built from. Verified: the only per-class manifest change against main is GsRefactoringEnvironment 23 -> 24 (+1 overall). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds two GemStone Explorer operations (catalog item R6): Rename Class Category and Rename Dictionary. Neither recompiles or commits — the user commits the session.
Rename Class Category
Right-click / pencil on a category node. Renames that single node of the dash-segmented category tree and carries its whole subtree along, server-side via
Class>>category:(queries/renameClassCategory.ts). Category names are compared by Symbol identity (asSymbol ==), neverString =, to dodge the 3.6.x wide-vs-narrowArgumentError 2718. Non-ASCII names are blocked below 3.7 (same boundary as server UTF-8 file-in). An empty overlay-only category is carried across in the client overlay.Rename Dictionary
Right-click / pencil on a dictionary node. A
SymbolDictionaryhas no name slot — its name is a self-referential entry it holds by identity — so the rename is a reflective swap (add the new self-entry, drop the old); the symbol-list index is unchanged and the classes it holds are untouched (queries/renameDictionary.ts). System dictionaries (Globals / Published / UserGlobals) and name collisions are refused. Source that names the dictionary literally is not rewritten (rare; the confirmation says so).Consistency with
mainBoth new queries resolve their dictionary through the shared
dictLookupExprhelper (from #397) rather than hand-rollingsymbolList at:/objectNamed:, so thenoHandRolledDictLookupguard test passes.dictLookupExpr's numeric branch is made uniformly nil-on-absent (matching its name branch) so an out-of-range index answers nil instead of raising.Review addressed (pre-PR)
A review of this branch was folded in before opening:
renamed: <n>; an unrecognised payload is an error, and 0-moved-while-classes-expected surfaces a warning, not false success.<old>-now moves with its subtree (>=).renamed: N skipped: M).classLookupExprindex-branch guard asymmetry.Tests
Unit tests for both queries and both command handlers (including the MED-2 warn/error paths and the MED-1 tab sweep), plus live-stone integration tests that drive real renames. Emitted Smalltalk is ASCII-only (a non-ASCII char in a doit trips the 3.6.x
ComStrmSetCursorcompiler bug). Full local suite: client 5267 · server 322 · mcp 92; lint / prettier / tsc clean.🤖 Generated with Claude Code