-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Reverse stacktrace display order #52430
base: master
Are you sure you want to change the base?
Conversation
I think the option to keep the old order should be in the IOContext (there is no need for it to be as course grained as command line args or environment variables) but open to other ideas. This should also get triage approval before merging. |
Triage likes this and recommends using an environment variable to override ordering (and check the ENV dict on each evaluation) |
To document the previous wish from a few triages ago for an environment variable: The reasoning is that this is much easier to control for deployment/CI than just an IOContext setting, which would then have to be special cased in code again anyway. I don't know if that was discussed this time around too. |
Okay, how bad is it going to be: @nanosoldier |
The package evaluation job you requested has completed - possible new issues were detected. |
I personally find this much harder to read. For me the relevant information is close to the source of the throw, now in the REPL I have to scroll down away from my current context to see that? The examples shown is exactly the kind of case where I think that inverted stack traces have much worse UX. I really think the error message should be close to the code. |
I also find this harder to read. I find the stack traces much more useful when the most relevant content (the error and closest stack frames) is near the most recent output/logs, at the top. |
More recent content is at the bottom, no? |
In long stack traces, you have to scroll back up to your code (or just press "up" to get the previous input to the REPL) - and worse, in very long stacktraces, neither your code nor the error message (in the current implementation) is visible at all if you hit your terminal output buffering limit. Either way, this is exactly why there is (going to be) a setting for this. Pretty much everyone on the triage call where this came up agreed on this being a better default, but there being some situations where the current behavior does still make sense. |
I am okay with making it changeable, but I am going to protest the UX default being changed.This behavior has been the default for a long while and as the proverb goes "there is no accounting for taste". I dislike that we keep adding environment variables and I feel like we should have a principled system for changing default behaviors for Base probably built around Preferences |
More precise language for what I meant: the most recent output/logs that precede the error currently appear near the top of the printed stack trace, and this is helpful for seeing as much relevant information as possible at once for diagnosis. |
Triage mentioned this favorably a few meetings ago—here's an implementation.
Here's an example of how this works
Master:
PR: