Repository navigation
Make getPriority optional with a default of 0 - #7
Merged
Merged
Conversation
Most dispatchable implementations don't overlap: isApplicableTo already
picks exactly one, so getPriority() { return 0; } was boilerplate.
Dispatchable is a type-only interface and can't carry a default body, so
the method is now optional and resolveDelegate and the generated
dispatcher treat a missing one as 0. Ties still throw, so two applicable
implementations without a priority remain an Ambiguous error.
The refactor skill now leaves getPriority out wherever it would return
0. The eval references drop it accordingly, and conventions.mjs reports
a missing priority as '-' and expects it for exclusive scenarios.
Says that the skill is opinionated about layout and naming, and that it can serve as a blueprint for refactoring existing code onto ts-implscan.
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.
Most
Dispatchableimplementations don't overlap:isApplicableToalready picks exactly one, sogetPriority() { return 0; }was boilerplate.Dispatchableis a type-only interface and can't carry a default body. Instead,getPriorityis now optional, andresolveDelegateand the generated dispatcher treat a missing one as0. Ties still throw, so two applicable implementations without a priority still raise the Ambiguous error. Existing implementations work as before.Changes
src/index.ts:getPriority?(): numbersrc/dispatcher.ts: when choosing an implementation and in error messages, a missing priority counts as0. The generated forwarder returnsgetPriority?.() ?? 0.resolveDelegateand dispatcher cases for implementations without a prioritycheck-layouttest that accepts implementations with and withoutgetPriorityPlainTextFormatterdrops its priority, and the README documents the default.getPriorityout wherever it would return0.getPrioritywherever it returned0conventions.mjsreports a missing priority as-Verification
npm run typecheck && npm test: 71 tests passevals/refactor-skill/scripts/validate.mjs: all 8 scenarios OKconventions.mjson the reference runs: 7/7 priorities match