refactor: expose Character.slugify/1 and Expression.parse_ref/1, delete copies#151
Merged
Merged
Conversation
…te copies
Two pieces of library knowledge were copy-pasted into the server and
would silently drift:
- slugify_name in server.ex was byte-for-byte the private slugify in
Character. If the library's slug rules ever changed, characters
created via build_start would be saved under slugs the library can
no longer derive from their names. Character.slugify/1 is now public
and documented as the single definition; the server copy is deleted.
- The node-ref regex existed three times: Expression.@ref_pattern,
a copy in Loader.parse_effect, and an anchored variant in
server.ex's parse_effect_target!. Expression.parse_ref/1 now parses
a whole-string single ref, and both call sites use it. The anchored
and leading variants used internally are derived from @ref_pattern
via Regex.source at compile time, so the pattern literal exists
exactly once in the codebase.
Behavior note: parse_ref is anchored, so Loader.parse_effect no longer
prefix-matches a target with trailing content ("...total_score + 1"
previously matched the leading ref and silently ignored the rest); it
now warns and skips the effect, consistent with the other unparseable-
target handling from #149. The malformed-contributes loader tests are
consolidated into one table-driven test covering all three cases.
Closes #124
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
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
1 participant
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.
Closes #124
What
Character.slugify/1is now public with doctests, documented as the single definition of the slug rules. The server's byte-for-byteslugify_namecopy is deleted — previously, if the library's slug rules changed, characters created viabuild_startwould become unfindable.Expression.parse_ref/1parses a whole-string single node ref, returning{:ok, {type, id, field}} | :error.Loader.parse_effectand the server'sparse_effect_target!both use it; the server's@effect_target_regexis deleted. The anchored/leading variants Expression needs internally are derived from@ref_patternviaRegex.sourceat compile time — the pattern literal now exists exactly once in the codebase.Behavior note
parse_refis anchored, soLoader.parse_effectno longer prefix-matches targets with trailing content:target = "ability('dexterity').total_score + 1"previously matched the leading ref and silently discarded the+ 1; it now warns and skips the effect, consistent with #149's unparseable-target handling. Covered in the (now table-driven) malformed-contributes loader test.Verification
parse_ref(valid ref, missing quotes, trailing content) andslugify.Summary by Bito