refactor(hashmap, set): remove the deprecated Show impls - #4199
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The changes are a small, mechanically verifiable refactor of Show punctuation output with no observed behavioral or API impact in the reviewed hunks.
Pull request overview
Refactors Show implementations for HashMap and linked-hash Set to write delimiters/separators via <+ template writing (including inline writer holes), with the goal of simplifying punctuation handling while keeping write_object dispatch monomorphic and preserving output.
Changes:
- Updated
Set[K]Show::outputto emit{,,, and}using<+instead ofwrite_string. - Updated
HashMap[K, V]Show::outputto emitHashMap::from_array([ ... ])and element tuple punctuation using<+, collapsing per-field punctuation into a single template string with writer holes.
File summaries
| File | Description |
|---|---|
| set/linked_hash_set.mbt | Switches Set Show punctuation writes ({, , , }) to <+ templates. |
| hashmap/utils.mbt | Switches HashMap Show punctuation and tuple formatting to <+ templates with inline writer holes. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Coverage Report for CI Build 6538Coverage increased (+0.02%) to 89.21%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
| @@ -337,18 +337,14 @@ pub impl[K : Show, V : Show] Show for HashMap[K, V] | |||
|
|
|||
| ///| | |||
| pub impl[K : Show, V : Show] Show for HashMap[K, V] with fn output(self, logger) { | |||
There was a problem hiding this comment.
should impl Show for HashMap be deprecated?
if it is maybe remove it now
There was a problem hiding this comment.
You're right - this Show impl is #deprecated (Debug migration). Per your suggestion I've turned the PR into a removal instead: Show for HashMap is deleted together with the hidden Show::{output, to_string} extends and the test that only exercised the deprecated Show. moon check is clean; hashmap 139/139 and set 70/70 pass.
| @@ -468,13 +468,13 @@ pub impl[K : Show] Show for Set[K] | |||
|
|
|||
| ///| | |||
| pub impl[K : Show] Show for Set[K] with fn output(self, logger) { | |||
There was a problem hiding this comment.
Same here - Show for Set was deprecated in favor of @debug.Debug; the impl, its hidden Show extends, and any test only exercising the deprecated Show are now removed. hashmap 139/139 and set 70/70 pass.
a9a3371 to
939b2f1
Compare
bobzhang
changed the title
<+> templates
`Show` for `HashMap` and linked-hash `Set` was deprecated in favor of
`@debug.Debug`. Drop the impls (and their hidden `Show::{output,
to_string}` extends plus the now-obsolete Show test), now that nothing in
the tree renders these collections through `Show` anymore.
Co-Authored-By: SeekMoon <seekmoon@moonbitlang.com>
939b2f1 to
853dd29
Compare
Removes the deprecated
Showimpls forHashMapand linked-hashSet(both were
#deprecatedin favor of@debug.Debug), together with theirhidden
Show::{output, to_string}extends and the hashmap test that onlyexercised the deprecated
Showrendering. Nothing in the tree rendersthese collections through
Showanymore, so the impls are simply gone(-85 lines).
Rewritten from the earlier
<+>-refactor version of this PR after reviewfeedback: polishing deprecated impls is wasted effort, removing them is
cleaner.
Verified:
moon check0 warnings/0 errors; hashmap 139/139, set 70/70.Generated with SeekMoon