Use blessings when possible for WPT UI

[mrobinson]

Use blessings when it is possible for the WPT UI and fall back to using
raw control characters. This also fixes a bug where the UI didn't work
correctly in iTerm.app. Remove the one line version of the UI, since it
is no longer used as the iTerm fallback.

[mrobinson]

@jdm or @jgraham r?

[frewsxcv]

Can we make this except ImportError just so we silence+catch all exceptions in case one occurs from something else?

[mrobinson]

The idea here was that if the import failed, if creating the Terminal failed, or if the __getattr__ call in blessings fails (which just calls into curses), we should always fall back. If you like I can split the work into two try/except blocks and print an error in the second one.

[jgraham]

try and except with no condition is never a good idea; even except Exception: is better because it won't catch KeyboardInterrupt. In this case I agree that splitting into two blocks is a better idea.

[jgraham]

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from the review on Reviewable.io

[mrobinson]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

tests/wpt/grouping_formatter.py, line 33 [r1] (raw file):
Done.

---

Comments from the review on Reviewable.io

[jgraham]

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 3 unresolved discussions.

---

tests/wpt/grouping_formatter.py, line 53 [r2] (raw file):
Factor out the default values here and below

---

tests/wpt/grouping_formatter.py, line 58 [r2] (raw file):
use Exception as exception (or I prefer Exception as e)

---

Comments from the review on Reviewable.io

[frewsxcv]

This probably needs a \n since write doesn't add one

[jgraham]

Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

tests/wpt/grouping_formatter.py, line 60 [r2] (raw file):
Actually I'm not sure this line doesn't do more harm than good since it causes a warning message that can't go through the mozlog infrastructure. I'd be inclined just to delete it.

---

Comments from the review on Reviewable.io

[mrobinson]

Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

tests/wpt/grouping_formatter.py, line 60 [r2] (raw file):
I think it's still useful since it allows us to improve the script if there are problems finding the appropriate control codes. If it's deleted, I believe that the two try/except blocks should be merged. There isn't any point in isolated ImportError when we respond to all exceptions in the same way.

---

Comments from the review on Reviewable.io

[mrobinson]

Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

tests/wpt/grouping_formatter.py, line 53 [r2] (raw file):
Done.

---

tests/wpt/grouping_formatter.py, line 58 [r2] (raw file):
Done.

---

Comments from the review on Reviewable.io

[jgraham]

Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

tests/wpt/grouping_formatter.py, line 60 [r2] (raw file):
I guess it's probably OK like this, but it's not ideal.

---

Comments from the review on Reviewable.io

[mrobinson]

Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

tests/wpt/grouping_formatter.py, line 60 [r2] (raw file):
If you have a better approach for sending the error through machformatter, I'll be happy to implement it. I considered recording it and logging it after the logger was initialized, but that seemed a bit convoluted.

---

Comments from the review on Reviewable.io

[jgraham]

@bors-servo: r+

[bors-servo]

📌 Commit 7225e36 has been approved by jgraham

[bors-servo]

⌛ Testing commit 7225e36 with merge d7c98f8...

[bors-servo]

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt

[salty-horse]

clear_eol and move_up are in the wrong order :)

[frewsxcv]

Excellent detective work @salty-horse 👍

[mrobinson]

Nice catch!

[frewsxcv]

@salty-horse You going to open a PR to fix it?

[salty-horse]

Will do. Can I put my other suggestion in a separate commit in the same PR?

[salty-horse]

instead of len(s.splitlines()), s.count('\n') should be equivalent (unless Windows does weird things, which I doubt)

[frewsxcv]

What advantages does your new solution have?

[salty-horse]

It's slightly faster (did not measure). No need to create a list with new strings just to count its length. However:

1. It only works if you assume the last line ends with a newline (Simple fix if you want to support other cases, but I don't think it's an issue)
2. Only tested on Linux.

[frewsxcv]

It might be better use s.count(os.linesep) for better Windows support, though I'm not too sure TBH

https://docs.python.org/2/library/os.html#os.linesep

[mrobinson]

This is counting the number of lines in self.current_display, which is always assigned internally. I believe that means that Windows line endings shouldn't pose any complications. As far as I can tell the .count("\n") approach should be fine.

[salty-horse]

The rest of the strings are composed with \n, so I'm not sure what that will do :)
If no-one can confirm it's working on Windows/Mac, I'll just patch the other bit.

[salty-horse]

I tested this on mac and windows, and it does what you expect. So seems fine to me:

'a\nb\nc\n'.count('\n')

[jdm]

I would expect splitting a constant string on a constant separator to work the same on all platforms :) The real question is given multiline string content from an external source, does it contain \r values that would cause difficulties when splitting on \n?

[salty-horse]

You're right. Not sure why I tested that :P. In any case, there is no external source other than file names in that output.