GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Create the side-by-side option (-y) feature for the diff command (Incomplete) by sami-daniel · Pull Request #117 · uutils/diffutils Conversation
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@ ## main #117 +/- ## ========================================== + Coverage 84.50% 86.19% +1.69% ========================================== Files 12 13 +1 Lines 5318 6216 +898 Branches 476 509 +33 ========================================== + Hits 4494 5358 +864 - Misses 809 839 +30 - Partials 15 19 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I'll fix the alerts before reopening the PR, 😁 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Is done : ) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Hi Sami. Thanks for your work on this feature! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This PR introduces a new string truncation utility (limited_string) with unit tests, and implements a side-by-side diff mode using the diff crate for output.
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/utils.rs | Added limited_string function and tests |
| src/side_diff.rs | Implemented side-by-side diff output using diff crate |
| src/params.rs | Added parameter handling for -y/--side-by-side flag |
| src/main.rs | Integrated side_diff module into main diff workflow |
| src/lib.rs | Re-exported side_diff diff function with an alias |
| src/diff.rs | Updated to include the new side_diff option |
src/side_diff.rs:22
// recalculate how many spaces are nescessary, cause we need to take into consideration the lenght of the word before print it.
src/lib.rs:15
pub use side_diff::diff as side_by_syde_diff;
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@sylvestre I think there is now a cool version of the side. Can you take a look? : ) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This PR introduces a side-by-side diff mode by adding a new "SideBySide" flag and related functionality, including a new utility function (limited_string), corresponding tests, and integration into the diff command using the diff crate.
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/utils.rs | Reordered import statements. |
| src/params.rs | Added SideBySide flag, width parameter, and updated default values and tests. |
| src/main.rs | Imported the new side_diff module. |
| src/lib.rs | Exposed side_diff module and re-exported its diff function as side_by_side_diff. |
| src/diff.rs | Added side-by-side diff branch handling in the main diff command function. |
| fuzz/fuzz_targets/fuzz_side.rs | Created a new fuzz target to test side-by-side diff behavior. |
| fuzz/Cargo.toml | Defined a new binary for the fuzz_side target. |
| .github/workflows/fuzzing.yml | Updated workflow to include the fuzz_side job (with a minor typo in the key). |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| File::create("target/fuzz.file.original") | ||
| .unwrap() | ||
| .write_all(&original) | ||
| .unwrap(); | ||
| File::create("target/fuzz.file.new") | ||
| .unwrap() | ||
| .write_all(&new) | ||
| .unwrap(); | ||
| File::create("target/fuzz.file") | ||
| .unwrap() | ||
| .write_all(&original) | ||
| .unwrap(); | ||
| File::create("target/fuzz.diff") | ||
| .unwrap() | ||
| .write_all(&output_buf) | ||
| .unwrap(); |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider removing disk writes from the fuzz target to avoid unnecessary I/O overhead and side effects during fuzzing. Using in-memory buffers for debugging outputs can improve iteration speed.
| File::create("target/fuzz.file.original") | |
| .unwrap() | |
| .write_all(&original) | |
| .unwrap(); | |
| File::create("target/fuzz.file.new") | |
| .unwrap() | |
| .write_all(&new) | |
| .unwrap(); | |
| File::create("target/fuzz.file") | |
| .unwrap() | |
| .write_all(&original) | |
| .unwrap(); | |
| File::create("target/fuzz.diff") | |
| .unwrap() | |
| .write_all(&output_buf) | |
| .unwrap(); | |
| let mut debug_original = Vec::new(); | |
| debug_original.extend_from_slice(&original); | |
| let mut debug_new = Vec::new(); | |
| debug_new.extend_from_slice(&new); | |
| let mut debug_diff = Vec::new(); | |
| debug_diff.extend_from_slice(&output_buf); |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't mind changing it, but it would be out of standard compared to other files related to fuzz.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
…omplete). - Create the function, in the utils package, limited_string that allows you to truncate a string based on a delimiter (May break the encoding of the character where it was cut) - Create tests for limited_string function - Add support for -y and --side-by-side flags that enables diff output for side-by-side mode - Create implementation of the diff -y (SideBySide) command, base command for sdiff, using the crate diff as engine. Currently it does not fully represent GNU diff -y, some flags (|, (, ), , /) could not be developed due to the limitation of the engine we currently use (crate diff), which did not allow perform logic around it. Only the use of '<' and '>' were enabled. - Create tests for SideBySide implementation
Create the diff -y utility, this time introducing tests and changes focused
mainly on the construction of the utility and issues related to alignment
and response tabulation. New parameters were introduced such as the size
of the total width of the output in the parameters. A new calculation was
introduced to determine the size of the output columns and the maximum
total column size. The tab and spacing mechanism has the same behavior
as the original diff, with tabs and spaces formatted in the same way.
- Introducing tests for the diff 'main' function
- Introducing fuzzing for side diff utility
- Introducing tests for internal mechanisms
- Modular functions that allow consistent changes across the entire project
Configuring CI to run fuzz from fuzz_side
|
excellent! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.