Enum pretty-printing #559
Merged
Enum pretty-printing #559
Conversation
This file contains 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
When we switched from representing enums as Rust enums to representing them as newtypes around numbers, the derived Debug implementation followed and instead of printing pretty names, we got just numbers. This commit stops deriving a Debug implementation and instead one is implemented manually. The new Debug impl uses the alternate() flag to decide between UPPER_CASE and CamelCase. By default, the UPPER_CASE representation that is now also used in the source is used. With the alternate() flag, CamelCase is used instead. This also prints bitmasks as combinations of the individual values. For example, ModMask::SHIFT | ModMask::LOCK actually is represented as "SHIFT | LOCK". This was not possible before when we used a Rust enum to represent these. Signed-off-by: Uli Schlachter <[email protected]>
|
Rebased on master |
psychon
force-pushed
the
pretty-printing2
branch
from
a073437 to
a3d0b4a
Compare
eduardosm
reviewed
Nov 21, 2020
Comment on lines
+93
to
+99
| let variants = [ | ||
| (Self::RAW_RECTANGLES.0.into(), "RAW_RECTANGLES", "RawRectangles"), | ||
| (Self::DELTA_RECTANGLES.0.into(), "DELTA_RECTANGLES", "DeltaRectangles"), | ||
| (Self::BOUNDING_BOX.0.into(), "BOUNDING_BOX", "BoundingBox"), | ||
| (Self::NON_EMPTY.0.into(), "NON_EMPTY", "NonEmpty"), | ||
| ]; | ||
| pretty_print_enum(fmt, self.0.into(), &variants) |
There was a problem hiding this comment.
Why not a match (or two depending on fmt)?
There was a problem hiding this comment.
Bad reason: I started writing the bit variant and then made the other version similar.
Slightly better reason, but I am not sure if this is true: #488. I hope that we only end up with one implementation of the code in the binary that just searches through some lookup table. With a match, I fear that the binary size would be larger. However, I didn't test this and it is just a theory.
Also: Two matches would mean more lines of code. :-)
Signed-off-by: Uli Schlachter <[email protected]>
Clippy complained about: useless conversion to the same type: `u32` Signed-off-by: Uli Schlachter <[email protected]>
error: very complex type used. Consider factoring parts into `type` definitions
--> src/utils.rs:286:14
|
286 | ) -> CallbackFormating<'a, 'b, fn(&mut Formatter<'_>, u32, &[(u32, &str, &str)]) -> Result>
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::type-complexity` implied by `-D warnings`
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity
Signed-off-by: Uli Schlachter <[email protected]>
psychon
force-pushed
the
pretty-printing2
branch
from
d3cb811 to
cf46b9a
Compare
In commit 7e4ea9c, I only fixed one of the two cases of "useless identity conversion". This commit also fixes the other half of the problem. Signed-off-by: Uli Schlachter <[email protected]>
eduardosm
approved these changes
Nov 21, 2020
|
Yay, I have just tested this and it works just as I wanted it 🙂 |
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.
This supersedes #555 and depends on #556 (the commit from that PR is included in this one).
CC #554 and @andrewshadura