Skip to content

[stacktrace] Visual improvements, expand causes by 1 by default #3790

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

Merged
merged 1 commit into from
Mar 19, 2025

Conversation

alexander-yakushev
Copy link
Member

@alexander-yakushev alexander-yakushev commented Mar 18, 2025

This is a much smaller fish to fry while we are working on #3789.

Two things here:

  • I've improved some spacing for cases when a cause is partially expanded with data vs with no data.
  • I made the "level 1" visibility the default for all clauses. Level 1 is where we show exception message and data. Level 0 is when we only show exception class. I don't think make sense to hide the message for nested causes. We even had complaints in the past where users missed the nesting part completely because of that.

Before

Example code: (throw (RuntimeException. "shrug" (ex-info "Foo" {:a 1} (ex-info "bar" {:a 2}))))

image ^ Causes 2 and 3 are completely collapsed by default. image ^ When level-1 expanded, the cause with data is missing an empty line between the next cause.

After

image ^ Message and data is visible by default for all causes. Empty line is present everywhere.

@alexander-yakushev
Copy link
Member Author

@yuhan0

@yuhan0
Copy link
Contributor

yuhan0 commented Mar 18, 2025

LGTM 👍

@bbatsov
Copy link
Member

bbatsov commented Mar 18, 2025

One general remark is that we still need a better way to fit the exception data (from ex-info) in this view, as it looks a bit of place to me. (a long standing issue, as it wasn't really a thing when the stracktrace viewer was developed)

@alexander-yakushev
Copy link
Member Author

Do you mean formatting it better or something else? The current formatting depends on the value of cider-print-fn. E.g. here's what happens if I set it to pprint:
image

Would you perhaps like the pretty-printing to be the default (regardless of cider-print-fn)?

Another thing is that the exceptions in cider-error buffer are clickable which brings the user into the inspector which now looks like this for the exceptions:

image

The user can navigate the data there using the normal inspector. But figuring out that you have to click/enter the exception is non-obvious.

@alexander-yakushev alexander-yakushev force-pushed the stacktrace-expand-1 branch 2 times, most recently from 24e31ae to e633401 Compare March 18, 2025 21:49
@bbatsov
Copy link
Member

bbatsov commented Mar 19, 2025

Do you mean formatting it better or something else? The current formatting depends on the value of cider-print-fn. E.g. here's what happens if I set it to pprint:

Yeah, more or less. Originally I was also thinking about 2 other things:

  • is this information should that should be displayed at all by default? I think the data is useful for tools, but not super useful for humans.
  • should it be possible to collapse/expand ex-info maps (e.g. those in your examples are quite big)

The main reason those are displayed as they are currently is that it was the easiest thing to do to accommodate them, but I always wondering what would be the optimal UI/UX for them.

Would you perhaps like the pretty-printing to be the default (regardless of cider-print-fn)?

I'm guessing in the context of stacktraces it makes sense to always pretty-print those, but I'm open to counter-arguments.

The user can navigate the data there using the normal inspector. But figuring out that you have to click/enter the exception is non-obvious.

Indeed. Even I forget about this from time to time now that I'm not doing as much Clojure as back in the day.

@alexander-yakushev
Copy link
Member Author

alexander-yakushev commented Mar 19, 2025

I think showing the data is useful. It's not only used for tooling to process (like the compilation errors) – e.g. I sometimes use the data map to attach some useful data to the exception in addition to writing it into the message.

I agree that the size of that data can be large. This is not a problem by default because by default pr is used to print the data, so it all goes into a single line (can be a damn long line but it doesn't matter much to the user).

Here's what we can do:

  • Use orchard.print to display the map unconditionally (same function as used for inline inspector values and the debugger). This will guarantee the data in a single line, and will abbreviate after a few values.
  • Make the printed data clickable which will also open the exception in the inspector, where the data can be inspected in a more convenient way. Making the data clickable is a better discovery method because it is more probable that the user instinctively mouses over the data than the exception class which is needed now to open the inspector.

@bbatsov
Copy link
Member

bbatsov commented Mar 19, 2025

I'm OK with your proposals.

@alexander-yakushev
Copy link
Member Author

Great, I'll proceed with that in subsequent PRs.

@alexander-yakushev alexander-yakushev merged commit d346f3e into master Mar 19, 2025
18 checks passed
@alexander-yakushev alexander-yakushev deleted the stacktrace-expand-1 branch March 19, 2025 10:22
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.

3 participants