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

Remove carriage return before each test line in spec reporter. #2401

Closed
wants to merge 1 commit into from

Conversation

Munter
Copy link
Member

@Munter Munter commented Jul 28, 2016

This PR replaces #2400

The spec reporter would output a carriage return char at the beginning of each line in non-TTY's and delete and overwrite the current line in TTY's.

Since this cursor call was only ever used at the beginning of a line it serves no purpose.

See discussion starting at https://gitter.im/mochajs/mocha?at=579a7573ac80b5ea3f149512

TODO:

  • Check phantomjs output
  • Before and after screenshots, TTY and non-TTY

@Munter
Copy link
Member Author

Munter commented Jul 28, 2016

Screenshot of TTY and non-TTY before:
image

Screenshot of TTY and non-TTY after:
image

@Munter
Copy link
Member Author

Munter commented Jul 28, 2016

opening non-TTY output in less before PR:
image

opening non-TTY output in less before PR:
image

@Munter Munter force-pushed the spec-reporter-carriage-return branch from 7eaef45 to 4d4390c Compare July 30, 2016 12:31
@boneskull
Copy link
Member

note that Travis-CI ran the full CI suite on SauceLabs from this PR; this only happens if the PR is from mochajs/mocha itself. that's why I wanted you to rebase.

anyway, I'm going to merge this manually into v3.0.0. it affects the output and thus could be construed as "breaking".

@boneskull
Copy link
Member

(appveyor failure is unknown though)

@boneskull
Copy link
Member

phantom looks ok

@boneskull
Copy link
Member

merged via 8741506

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.

None yet

2 participants