Parameterize indent step in serlialize::json::PrettyEncoder #16601
Conversation
|
Could you add a test for this as well? |
|
|
||
| /// Creates a new encoder that indents `indent` number of spaces for each level | ||
| pub fn with_indent<'a>(writer: &'a mut Writer, indent: uint) -> PrettyEncoder<'a> { | ||
| PrettyEncoder { writer: writer, curr_indent: 0, indent: indent, } | ||
| } |
There was a problem hiding this comment.
This may be better done with a setter rather than a constructor in case more configuration is added later on.
There was a problem hiding this comment.
A public setter? That could possibly allow the user to set a different indent in the middle of encoding. If the indent is larger than the previous value, then any
self.curr_indent -= self.indent;
has the potential to underflow. Would that just need to be a documentation warning then?
|
@alexcrichton That's just it, I'm not sure what to test here. Maybe something that generates some JSON structures with different levels and checks the number of spaces at the beginning of each line? |
|
@alexcrichton I added a test for This is with a clean build from |
|
That test looks great, thanks! I would still favor a configuration method with a warning about underflow (changing while serializing), however. |
|
@alexcrichton Okay, I have changed it to a setter. I figured out how to make it safe to set during encoding so we don't need a warning. And the test passes on my Linux machine. |
Previously, `PrettyEncoder` indented a magic constant of 2 spaces per level, which may be fine for most uses but in my use case I would like to allow the user to specify the indent step for the outputted JSON in my program. This is small change that does not break any existing code whatsoever, and does not change the behavior of existing uses. `PrettyEncoder::new()` still uses the default of 2. I couldn't think of any simple tests for this change. The obvious one would be to check the outputted JSON for the correct number of spaces per indent level, but I think that would be more complex than the actual change itself and test little besides correctness and consistency, which can be verified visually. There's already a test for correct parsing of pretty-printed JSON that should still pass with this change.
fix: checkout repo before run typos I see some typos at lastest master :(
Previously,
PrettyEncoderindented a magic constant of 2 spaces per level, which may be fine for most uses but in my use case I would like to allow the user to specify the indent step for the outputted JSON in my program.This is small change that does not break any existing code whatsoever, and does not change the behavior of existing uses.
PrettyEncoder::new()still uses the default of 2.I couldn't think of any simple tests for this change. The obvious one would be to check the outputted JSON for the correct number of spaces per indent level, but I think that would be more complex than the actual change itself and test little besides correctness and consistency, which can be verified visually. There's already a test for correct parsing of pretty-printed JSON that should still pass with this change.