Improve display of continuation lines with multiline errors#1807
Improve display of continuation lines with multiline errors#1807nicoddemus merged 1 commit intopytest-dev:masterfrom
Conversation
|
Hey @blueyed. thanks! Looks good to me, perhaps though we should add a |
| prefix = ' ' | ||
| tw.line(prefix + line.strip(), red=True) | ||
| if lines: | ||
| tw.line('E ' + lines[0].strip(), red=True) |
There was a problem hiding this comment.
Just noticed that we have this in py._code.code.py:
class FormattedExcinfo(object):
""" presenting information about failing Functions and Generators. """
# for traceback entries
flow_marker = ">"
fail_marker = "E"Could please change the code use those constants instead?
There was a problem hiding this comment.
Done. Not sure though if that is really good like that.
After all it should be some more generic method probably for this kind of messages?
|
LGTM 👍 |
29ec412 to
998f9a5
Compare
Fixes pytest-dev#717. Follow-up to pytest-dev#1762.
998f9a5 to
c163cc7
Compare
|
@nicoddemus |
|
Thanks for the heads up @blueyed! |
|
@blueyed said:
That's probably by bad, I was advising new contributors at the Europython sprint 😬 Looking again at #717 (which #1762 attempts to fix) it's more of a "bring into line" (with other traceback output), than strictly a "bugfix". So I guess default to features branch for these kind of tickets? |
|
@tomviner not sure, that's such a small change in output, I think it is OK for it to go to master. Others may have other opinions though. 😁 |
|
I'm still wondering what crazy output parsing people do (like for IDEs) though, and how much we break even with small output changes. |
|
@The-Compiler I'm sure they do all sorts. But if we have faith in the longevity of pytest, perhaps being more consistent now will help future crazy output parsers ;-) |
|
@tomviner Sure, but I think that's a good reason to have any changes to the output in |
|
@The-Compiler see what you mean. So really, for patches that alter the pytest interface, only changes that were obviously broken before should be aimed at master. |
Fixes #717.
Follow-up to #1762.