[common] Add {:r} repr formatter for TypeSafeIndex and Identifier - #24936
[common] Add {:r} repr formatter for TypeSafeIndex and Identifier#24936castor639 wants to merge 1 commit into
Conversation
Default fmt output remains the bare integer. The new {:r} specifier
prints a typed representation (e.g. FooIndex(0), FooId(1)) using
NiceTypeName. Also teach NiceTypeName to canonicalize TypeSafeIndex
the same way it already does for Identifier.
Fixes RobotLocomotion#24262.
SeanCurtis-TRI
added
the
release notes: feature
|
+(release notes: feature) +a:@SeanCurtis-TRI for feature review. |
SeanCurtis-TRI
left a comment
SeanCurtis-TRI
left a comment
There was a problem hiding this comment.
This all makes sense as far as it goes. I think we might want to discuss a bit more if this is exactly the format string we want -- see the questions raised in the linked issue. We'll stall until that discussion is definitively resolved.
@SeanCurtis-TRI reviewed 7 files and all commit messages, and made 2 comments.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee SeanCurtis-TRI(platform), needs at least two assigned reviewers (waiting on castor639).
common/identifier.h line 293 at r1 (raw file):
} private:
nit: This goes against drake style guide. The member should neither be private nor should it have a trailing underscore.
|
@drake-jenkins-bot test this please |
|
@drake-jenkins-bot retest this please |
|
@drake-jenkins-bot retest this please |
|
Woops, it's just docstrings failing CI, I thought we had bad timing on the infrastructure issue yesterday. Sorry about that! |
jwnimmer-tri
left a comment
jwnimmer-tri
left a comment
There was a problem hiding this comment.
@jwnimmer-tri made 1 comment.
Reviewable status: 1 unresolved discussion, LGTM missing from assignees jwnimmer-tri(platform),SeanCurtis-TRI(platform) (waiting on SeanCurtis-TRI).
common/identifier.h line 293 at r1 (raw file):
Previously, SeanCurtis-TRI (Sean Curtis) wrote…
nit: This goes against drake style guide. The member should neither be private nor should it have a trailing underscore.
It's conventional for format specs to be private (see fmt_eigen.h, and all upstream fmt formatters).
However, yes, the member field should not be named with trailing underscore.
4 participants
Default fmt output remains the bare integer. The new {:r} specifier prints a typed representation (e.g. FooIndex(0), FooId(1)) using NiceTypeName. Also teach NiceTypeName to canonicalize TypeSafeIndex the same way it already does for Identifier.
Fixes #24262.
This change is