Selection: report a bare word that names nothing (#124) - #125
Merged
Conversation
`:color red not heliix` painted the whole structure and exited 0. A typo'd selector produced an empty selection, and `not` inverted that into everything — the same figure-shaped failure as #116 and #121, from the same function. #121's end-of-input check could not see it: a single unknown word leaves no leftover token. The obstacle is that the parser is handed one MolObject at a time while callers loop over objects, so `1crn` parsed against `1ubq` is a legitimate cross-object reference that must yield empty *without* an error. A typo and a not-this-object name are indistinguishable at that point. So give the parser the missing half: a caller-supplied `knownObject` lookup, bundled with the `$name` resolver into `Selection::ParseEnv` and built once by `Application::selectionParseEnv()` off the whole ObjectStore. A word is a typo only when nothing anywhere answers to it. Without the lookup the parser stays silent rather than guessing — a caller that cannot enumerate objects has no business accusing one. Applies to the bare `<obj>` form, `obj <name>` (which was silent while the bare spelling errored — the odd half of the pair) and `<name>/*`. Unknown selectors are reported ahead of an unbalanced paren, since the typo is a definite fact and the paren is often its symptom, and the leftmost one wins. Also fixes object names containing '-' or '.': `readWord` split `my-model` into `my`, so it never resolved and #121 reported `unexpected '-'`. The bare-object branch now re-reads under object-name rules, as the digit-led path already did, and a typo of one is reported whole rather than as its first fragment. Still silent, and listed as such in docs/SELECTIONS.md: unresolved `$name`, slash paths, `<obj>/(...)`, unknown imgt regions, and a typo absorbed into a value list (`chain A heliix`). Those need rules of their own, not this lookup. Verified no false positives: cross-object barewords under both scopes, digit- led and hyphenated names, `<obj>/(...)`, slash forms, `$name`, disabled objects, objects in other tabs, the shipped lib recipes, and the 59-expression corpus, which stays byte-identical to the pre-#116 baseline. Fixes #124
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.
Fixes #124.
The bug
A typo'd selector produced an empty selection, and
notinverted that intoeverything. Same figure-shaped failure as #116 and #121, from the same
function — and #121's end-of-input check couldn't see it, because a single
unknown word leaves no leftover token to trip on.
Why it wasn't a one-liner
The parser is handed one
MolObjectat a time while callers loop overobjects.
1crnparsed against1ubqis a legitimate cross-object referencethat must yield empty without an error, so the loop can go on to reach
1crn. At single-object parse time a typo and a not-this-object name areindistinguishable — which is exactly why the branch returned empty.
So the fix supplies the missing knowledge rather than guessing: a caller-side
knownObjectlookup, bundled with the$nameresolver into aSelection::ParseEnvand built once byApplication::selectionParseEnv()offthe whole
ObjectStore. A word is a typo only when nothing anywhere answersto it. With no lookup supplied the parser stays silent — a caller that can't
enumerate objects has no business accusing one, and that default is what keeps
the convenience overload from inventing false positives.
Covers the bare
<obj>form,obj <name>— which was silent while the barespelling errored, the odd half of the pair — and
<name>/*. Unknown selectorsare reported ahead of an unbalanced paren (the typo is a definite fact; the
paren is often its symptom), leftmost first.
A second bug fixed on the way
Object names containing
-or.never resolved:readWordsplitmy-modelinto
my, so:count my-modelfailed withunexpected '-'. The bare-objectbranch now re-reads under object-name rules, as the digit-led path (
1ubq)already did — and a typo of one is reported whole (
my-modelx) instead of asits first fragment (
my).Verification
False positives were the main risk, so I went looking rather than assuming.
Confirmed still silent and correct: cross-object barewords under both scopes,
digit-led (
1ubq) and hyphenated/dotted names,<obj>/(...),<obj>/*,/obj/chain/resi/name, resolved and unresolved$name, disabled objects,objects in other tabs,
:foreach ... in <sel>,centroid()/pca()registerexpressions,
:label corner topleft = ..., andlib/residue_scan.mtend-to-end. The 59-expression corpus stays byte-identical to the pre-#116
baseline. 317 selection assertions, 55 script assertions, all 6 suites green.
Scope, stated honestly
These still match nothing silently, so
not <them>still inverts toeverything — they need rules of their own, not this lookup, and
docs/SELECTIONS.mdnow says so in a table rather than implying the class isclosed:
$namenot $typonot /nosuch/A/10<obj>/(...)not nosuchobj/(chain A)imgtregionnot imgt nosuchregionchain A heliix(read as a second chain id)The last is the biggest remaining hole and the one closest to #124's shape —
:color red chain A heliixpaints all of chain A and reports success. It can'tuse
knownObject(chain ids are opaque), but theimgtbranch already voidsthe whole term on an unknown region name, so there's precedent for stricter
list handling in the same function.
🤖 Generated with Claude Code