-
Notifications
You must be signed in to change notification settings - Fork 139
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
Basic diff output #238
base: master
Are you sure you want to change the base?
Basic diff output #238
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
use similar::TextDiff; | ||
use std::path::Path; | ||
|
||
use crate::error::{Error, Result}; | ||
|
||
pub(crate) fn create_udiff<UO: AsRef<[u8]>, UN: AsRef<[u8]>>( | ||
old: UO, | ||
new: UN, | ||
context: usize, | ||
path: Option<&Path>, | ||
) -> Result<String> { | ||
let diff = TextDiff::from_lines(old.as_ref(), new.as_ref()); | ||
let path = if let Some(path) = path { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should just convert to a lossy string here, not sure its worth erroring out. Having the user see squareish symbols is better than not being able to see a diff at all There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed when acting as an end user interface, not so when the output is used as a proper diff. Or I'm overestimating the latter use case? |
||
if let Some(spath) = path.to_str() { | ||
spath | ||
} else { | ||
return Err(Error::InvalidPath(path.into())); | ||
} | ||
} else { | ||
"" | ||
}; | ||
|
||
let mut udiff = diff.unified_diff(); | ||
udiff.header(path, path); | ||
udiff.context_radius(context); | ||
|
||
Ok(udiff.to_string()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think diff is short enough to not warrant a short flag,
-d
might be more useful for other things in the futureThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about
-D
? Even sed doesn't have many flags. (I impl'd this after all ;)And what do you think about an option specifying context radius? And filename for piped diffing?