SwitchSlot: remove Display implementation#10015
Merged
jgallagher merged 2 commits intomainfrom Mar 12, 2026
Merged
Conversation
jgallagher
commented
Mar 10, 2026
| Span::styled(port.to_string(), ok_style), | ||
| Span::styled(" on switch ", label_style), | ||
| Span::styled(switch.to_string(), ok_style), | ||
| Span::styled(switch_description, ok_style), |
Contributor
Author
There was a problem hiding this comment.
This should be the only behavioral change; wicket will now display
* Port $NAME on switch 0
instead of
* Port $NAME on switch switch0
But if that feels like a regression I can change this back.
Contributor
There was a problem hiding this comment.
I think the new version is easier to read
Contributor
There was a problem hiding this comment.
Thoughts on * Port $NAME on switch0?
Contributor
Author
There was a problem hiding this comment.
🤷 I don't feel strongly about on switch 0 vs on switch0. Both are much nicer than on switch switch0. 😅
Contributor
There was a problem hiding this comment.
Oh I flipped around the before and after haha -- this is great
internet-diglett
approved these changes
Mar 10, 2026
sunshowers
approved these changes
Mar 10, 2026
jgallagher
force-pushed
the
john/switch-slot-no-display
branch
from
1693aec to
6e4dd7c
Compare
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
3 participants
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.
Reasoning here is similar to #10010: this isn't really a naturally-displayable type, and in this case we were previously abusing this to stringify it both in the db (fixed by #9984) and the external API (fixed by #10014, on which this PR is based).
Maybe worth noting: I expected this to be entirely futureproofing, but by doing this I found a few external API types that I'd missed in an earlier draft of #10014 (types where we stringified
SwitchSlots for output but never parsed them as input).All our remaining uses of the display impl are either for logging or constructing internal error messages; I changed all of these to use
Debug, and added a manualDebugimplementation that matches the oldDisplayimplementation. This should keep the error messages and logs consistent while not leading us to the pit of sadness of assuming we can format and parse these values as strings without consideration.