Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add clear_fmt_recursive method #45

Merged
merged 3 commits into from
Jun 10, 2022
Merged

Conversation

nicopap
Copy link
Contributor

@nicopap nicopap commented May 16, 2022

When comparing two different KdlNode or KdlDocument, it's useful to
have a "canonical" representation where formatting differences do not
matter.

clear_fmt removes all formatting from a KdlNode, but does not apply
recursively, as a result, it doesn't create a canonical representation.
clear_fmt_recursive solves this by applying clear_fmt recursively to
the contents of the node.

@nicopap nicopap marked this pull request as draft May 16, 2022 08:46
When comparing two different `KdlNode` or `KdlDocument`, it's useful to
have a "canonical" representation where formatting differences do not
matter.

`clear_fmt` removes all formatting from a `KdlNode`, but does not apply
recursively, as a result, it doesn't create a canonical representation.
`clear_fmt_recursive` solves this by applying `clear_fmt` recursively to
the contents of the node.
@nicopap nicopap marked this pull request as ready for review May 16, 2022 09:01
@nicopap
Copy link
Contributor Author

nicopap commented Jun 10, 2022

@zkat May I get your attention? You might have overlooked this PR.

@zkat
Copy link
Member

zkat commented Jun 10, 2022

Oh sorry, I did miss this.

So what you want here is to use something that will wipe out comments, not just format everything into a "standard" format?

@nicopap
Copy link
Contributor Author

nicopap commented Jun 10, 2022

Yeah, I'd like to whip out comments. But it's not because I want to print it, it's more to do with equality and testing.

The idea is to have a representation that only depends on the actual values of the nodes. It's useful for two things: testing and comparing two different KdlNode.

The #[derive(PartialEq)] means that when comparing two KdlNode, even formatting differences will make them unequal. A method to get rid of all formatting (recursively) enables comparing them independently of formatting. The current clear_fmt is not enough, since nested nodes will not be "deformatted" and will be unequal despite equal values.

It's also useful for testing. The output of assert_eq! (even from the pretty_assertions crate) is difficult to read visually when comparing KdlNode, because there is all the formatting-related fields on the debug output. Simply translating the KdlNode to a "deformatted" string makes the diff much easier to read.

I wrote my own clear_fmt_recursive for my tests here https://github.com/nicopap/bevy-kdl-ui/blob/2ba1833f38178fb7ddfbf39f203be6a766fe59bc/template-kdl/tests/readme-examples.rs#L82-L83 and it helped me fix my own bugs much quicker.

@zkat zkat merged commit bfe0682 into kdl-org:main Jun 10, 2022
@zkat
Copy link
Member

zkat commented Jun 10, 2022

sgtm :)

zkat pushed a commit that referenced this pull request Jun 11, 2022
Fixes #46

When comparing two different `KdlNode` or `KdlDocument`, it's useful to
have a "canonical" representation where formatting differences do not
matter.

`clear_fmt` removes all formatting from a `KdlNode`, but does not apply
recursively, as a result, it doesn't create a canonical representation.
`clear_fmt_recursive` solves this by applying `clear_fmt` recursively to
the contents of the node.
@zkat
Copy link
Member

zkat commented Jun 11, 2022

this has been published as part of [email protected] :)

So great that you're using it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants