8000 Make Whitespace differences more visually identifiable by gowerc · Pull Request #99 · gowerc/diffdf · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Make Whitespace differences more visually identifiable #99

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Jul 24, 2024
Merged

Make Whitespace differences more visually identifiable #99

merged 9 commits into from
Jul 24, 2024

Conversation

gowerc
Copy link
Owner
@gowerc gowerc commented Jul 8, 2024

Closes #87

@gowerc gowerc requested a review from kieranjmartin July 8, 2024 14:56
@github-actions GitHub Actions
Copy link
Contributor
github-actions bot commented Jul 8, 2024 •
edited
Loading

badge

Code Coverage Summary

Filename                Stmts    Miss  Cover    Missing
--------------------  -------  ------  -------  ----------------
R/ascii_tables.R          105      19  81.90%   10, 149-170, 212
R/cast_variables.R         49       0  100.00%
R/diffdf.R                137      18  86.86%   281-298, 325
R/generate_keyname.R       10       1  90.00%   16
R/identify.R              132      10  92.42%   36, 245, 282-289
R/is_different.R           52       0  100.00%
R/issuerows.R              40       0  100.00%
R/issues.R                 13       1  92.31%   51
R/misc_functions.R         14       2  85.71%   9, 13
R/print.R                  23       0  100.00%
TOTAL                     575      51  91.13%

Results for commit: 19b47a1

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions GitHub Actions
Copy link
Contributor
github-actions bot commented Jul 8, 2024 •
edited
Loading

Unit Tests Summary

  1 files   10 suites   5s ⏱️
 41 tests  40 ✅ 1 💤 0 ❌
550 runs  547 ✅ 3 💤 0 ❌

Results for commit 19b47a1.

♻️ This comment has been updated with latest results.

8000
@gowerc
Copy link
Owner Author
gowerc commented Jul 9, 2024 •
edited
Loading

Note that this is currently broken due to the current snapshot tests failing. These snapshots are currently manually implemented so rather than adjusting them I think we should wait until #101 is merged and then convert them to use testthats snapshot frame work.

For reference the failing tests can be found at: test-print_output.R:42:9

@gowerc
Copy link
Owner Author
gowerc commented Jul 9, 2024

This ended up being slightly more complicated than expected. The crux of the issue was that it turns out most of our character conversion was being done implicitly via as.matrix() via the use of the apply() function. The nature of this change meant that I needed to replace this with explicit character conversion which required a overhaul of the as_cropped_ascii function for class specific methods.

kieranjmartin
kieranjmartin reviewed Jul 16, 2024
View reviewed changes
kieranjmartin
kieranjmartin previously approved these changes Jul 18, 2024
View reviewed changes
@gowerc gowerc merged commit 78f2498 into master Jul 24, 2024
23 checks passed
@gowerc gowerc deleted the 87-display-whitespace branch July 24, 2024 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewer 5CF4 s

@kieranjmartin kieranjmartin kieranjmartin left review comments

Assignees

@kieranjmartin kieranjmartin

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

issues with white space and new lines
2 participants
@gowerc @kieranjmartin
0