difference.rs
difference.rs copied to clipboard
johannhof
No color options, print support for windows
This PR was made to address issues in -> https://github.com/johannhof/difference.rs/issues/15
First rust contribution so I'll preface to say if things seem messed up/wrong let me know and I'll try my best to fix them!
- Added support --word-diff flag. If that flag is set, then instead of colors, text will be used to distinguish diffs in the text.
- Added a new 'constructor' that takes in an options object. (Just word-diff for now, but eventually this can hold a bunch of options)
- Added options struct that will hold options that difference.rs might use later
- Un deprecated print_diff. This was done because AFAIK the term module only supports stdin and stdout. So to leverage this library to support windows, I took the avenue to have a function that will print directly to stdin and stdout so other terminals are supported.
Appears travis is failing on nightly due to a dep or sub dep not being able to compile?
Hi there, thanks for the contribution. Travis is failing because Clippy seems to be borked on the latest Nightly again, nothing to worry about. If it compiles again and we find that we missed some lint, we'll just push the fix afterwards.
This looks good from a quick glance, but I'll get to reviewing later today.
Hey, thanks again for the PR. Sorry for taking so long to get back to you.
I have two concerns with this patch. I don't think we should introduce term as a dependency and I would like to avoid un-deprecating print_diff. IMO the library should focus on computing the difference and not pretty-printing the results.
Maybe we should just make it really simple and do word_diff if cfg!(windows).
Hm, I can understand the concern the concern about additional dependencies.
Whithout knowing the details of this PR .. what if the additional dependency was behind some kind of feature flag?
I'm not sure why consumers of this library can't just depend on term instead and use one of the formatting examples I provided. It shouldn't be a big effort, the formatting code isn't that large and it gives consumers much more flexibility.
IMO the library should focus on computing the difference and not pretty-printing the results.
In that case, this crate should simply print +/- without any color, so that it produces reliably and reasonable result across all platforms.
Having it produce silly result on Windows by default, or it produce different result based on platforms would hurt consumers of this crate in different ways.
Having it produce silly result on Windows by default, or it produce different result based on platforms would hurt consumers of this crate in different ways.
I can see that there are edge cases where you'd want the print output to be exactly the same across platforms, but would this really be such a big pain on consumers to implement their on printing code if they have such requirements? Maybe I'm just underestimating the use case for having consistent output on all platforms.
In that case, this crate should simply print +/- without any color, so that it produces reliably and reasonable result across all platforms.
Considering that colorful output (only for some consumers unfortunately) has been the default for some time, would you consider an optional nocolor compiler flag, that makes the behavior consistent, an acceptable solution?
I can see that there are edge cases where you'd want the print output to be exactly the same across platforms, but would this really be such a big pain on consumers to implement their on printing code if they have such requirements? Maybe I'm just underestimating the use case for having consistent output on all platforms.
It may not be a big pain to implement that, but it may give downstream tools and their users false expectation if they don't check behavior on different platform carefully.
For example, a developer on Windows may think this crate would reliably produce text-based diff, so that users can use commandline tools to consume its output, however it is not true on other platforms. This can happen vice versa. It would just make people confusing.
I think Display should have reliable behavior. If you think this should just be a best-effort thing, this should probably be done in Debug instead.
Considering that colorful output (only for some consumers unfortunately) has been the default for some time, would you consider an optional nocolor compiler flag, that makes the behavior consistent, an acceptable solution?
A reliable behavior should be the default, I think. Wouldn't it be fine to just implement the coloring in the commandline tool, but leave the lib part generate text result?
Alternatively, you can probably add an optional default feature with term dependency to generate reliable colored result on all platforms, so that it keeps the original default behavior while still allows users to remove that dependency.