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 HTML side by side (diff) format #102

Merged
merged 2 commits into from
Apr 25, 2022

Conversation

kwridan
Copy link
Contributor

@kwridan kwridan commented Apr 10, 2022

Describe your changes

Main feature:

  • Adding a new html format htmlSideBySide that displays results in a diff like format
  • This can sometimes be easier to read through, espically when comparing differences in common values

Supporting changes:

  • HTML escpae utility was pulled out to its own extension to allow sharing between both html renderers
  • A small fix was made to the Makefil to enable regenerating tests

TODO:

  • Iterate on styles, are there any better options?
  • Are there any alternate format names that are more suitable (e.g. htmlDiff)?

Testing performed

  • Generate a diff using the new side by side format and examine the results
swift build
LOCAL_XCDIFF=$(pwd)/.build/debug/xcdiff

cd Fixtures
$LOCAL_XCDIFF --format htmlSideBySide --verbose > results.html

open results.html

Screenshots

v2-diff-1

v2-diff-2

@codecov
Copy link

codecov bot commented Apr 10, 2022

Codecov Report

Merging #102 (b819269) into main (3fef652) will increase coverage by 0.09%.
The diff coverage is 97.28%.

@@            Coverage Diff             @@
##             main     #102      +/-   ##
==========================================
+ Coverage   96.33%   96.43%   +0.09%     
==========================================
  Files          47       49       +2     
  Lines        2400     2719     +319     
==========================================
+ Hits         2312     2622     +310     
- Misses         88       97       +9     
Impacted Files Coverage Δ
Sources/XCDiffCore/ProjectComparator.swift 97.72% <ø> (ø)
...tResultRenderer/HTMLSideBySideResultRenderer.swift 97.14% <97.14%> (ø)
Sources/XCDiffCore/Library/String+Extensions.swift 100.00% <100.00%> (ø)
...iffCore/ResultRenderer/Renderer/HTMLRenderer.swift 100.00% <100.00%> (ø)
...ore/ResultRenderer/Renderer/String+HTMLUtils.swift 100.00% <100.00%> (ø)
...fCore/ResultRenderer/UniversalResultRenderer.swift 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a82777...b819269. Read the comment docs.

Main feature:
- Adding a new html format `htmlSideBySide` that displays results in a diff like format
- This can sometimes be easier to read through, espically when comparing differences in common values

Supporting changes:
- HTML escpae utility was pulled out to its own extension to allow sharing between both html renderers
- A small fix was made to the Makefil to enable regenerating tests

TODO:

- [ ] Iterate on styles, are there any better options?
- [ ] Are there any alternate format names that are more suitable (e.g. `htmlDiff`)?

Test Plan:

- Generate a diff using the new side by side format and examine the results

```sh
swift build
LOCAL_XCDIFF=$(pwd)/.build/debug/xcdiff

cd Fixtures
$LOCAL_XCDIFF --format htmlSideBySide --verbose > results.html

open results.html
```

Signed-off-by: Kassem Wridan <[email protected]>
@kwridan kwridan force-pushed the feature/html-diff-side-by-side branch from b5fab42 to 842753f Compare April 10, 2022 18:38
- Added grey background color for empty cells
- Added a different yellow color for value mismatch keys
- Removed "Value mismatch" label and replaced with a spacer

Signed-off-by: Kassem Wridan <[email protected]>
Copy link
Contributor

@marciniwanicki marciniwanicki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great 👍

@kwridan
Copy link
Contributor Author

kwridan commented Apr 25, 2022

@marciniwanicki did you have any preferences on what to call the new format? is htmlSideBySide ok? Feels like a mouthful 😅

@marciniwanicki
Copy link
Contributor

@kwridan no preference, I like htmlSideBySide. In the future we can perhaps add a bit of JavaScript and unify those 2, i.e. a single document could be dynamically switched between single and split layout.

@kwridan kwridan marked this pull request as ready for review April 25, 2022 13:46
@kwridan kwridan merged commit 2ad1abb into bloomberg:main Apr 25, 2022
@kwridan kwridan deleted the feature/html-diff-side-by-side branch April 25, 2022 13:48
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.

2 participants