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

Reporter: TapReporter align actual with expected. #107

Merged
merged 2 commits into from Aug 5, 2020

Conversation

rwjblue
Copy link
Contributor

@rwjblue rwjblue commented May 19, 2018

When comparing two strings, it is significantly easier to spot the difference when they are aligned. This change pads the label for actual with a couple of spaces so that the actual value is printed at the same indentation as the expected value. Previously the expected value was right shifted by 2 characters...

Before:

2018-05-18_23-31-32_65aka_tmux image

After:

2018-05-18_23-29-33_ogp95_tmux image

When comparing two strings, it is significantly easier to spot the
difference when they are aligned. This change pads the label for
`actual` with a couple of spaces so that the actual value is printed at
the same indentation as the expected value. Previously the expected
value was right shifted by 2 characters...
@rwjblue
Copy link
Contributor Author

rwjblue commented Jun 21, 2018

Thoughts?

Copy link
Member

@trentmwillis trentmwillis left a comment

Choose a reason for hiding this comment

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

Must've overlooked this...

Overall, I like it, but I'm wondering if we should align the other fields as well? Or, maybe put the extra space on the other side of :? Seems kind of odd to me that we force some of the : to line up but not others.

@jzaefferer
Copy link
Contributor

This certainly looks like an improvement!

@Krinkle Krinkle merged commit 8180357 into js-reporters:master Aug 5, 2020
@rwjblue rwjblue deleted the align-actual-with-expected branch August 5, 2020 22:20
Krinkle added a commit to qunitjs/qunit that referenced this pull request Sep 8, 2020
Relevant changes:

* Reporter: Align `actual` with `expected` in TapReporter. (Robert Jackson)
  js-reporters/js-reporters#107
Krinkle added a commit to qunitjs/qunit that referenced this pull request Sep 8, 2020
Relevant changes:

* Reporter: Align `actual` with `expected` in TapReporter. (Robert Jackson)
  js-reporters/js-reporters#107
Krinkle added a commit to qunitjs/qunit that referenced this pull request Sep 10, 2020
Relevant changes:

* Reporter: Align `actual` with `expected` in TapReporter. (Robert Jackson)
  js-reporters/js-reporters#107
Krinkle added a commit to qunitjs/qunit that referenced this pull request Sep 10, 2020
Relevant changes:

* Reporter: Align `actual` with `expected` in TapReporter. (Robert Jackson)
  js-reporters/js-reporters#107
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants