Update stacktrace to include trimmed? and use color? in web

[sirmspencer]

• I expanded the use of the color? option to show colors on the html response for (wrap-stacktrace-web ) based on the error type (java, clojure, etc). This uses the same logic for determining color as (wrap-stacktrace-log).

• I also added a new tag trimmed? to use the :trimmed-elems instead of :trace-elems. :trimmed-elems is a shorter stack trace provided the error report. You can see an example in current code by comparing the log output and the web output for a :cause. The log output already uses :trimmed-elems in pst-on.

closes #386

[weavejester]

Thanks for the commits. Can you just upload some screenshots of before and after so there's a record of what you're doing? It's hard to tell from the description alone.

Can you also make sure the PR adheres to the contributing guidelines, particularly the parts around commit message, changes unrelated to the purpose of the PR, and line length.

[sirmspencer]

Default web view (existing view). The stack trace continues past the viewable screen.

This is the web view with color? and trimmed? = true

[sirmspencer]

Default log view (existing). The stack trace continues past the visible screen.

Here is the log view with color? and trimmed? = true

[sirmspencer]

Thanks for the commits. Can you just upload some screenshots of before and after so there's a record of what you're doing? It's hard to tell from the description alone.

I cant fit screen shots of before, the stack traces are very long. That would be the main reason I created this PR, to shorten the error reporting to a screen readable size.

[weavejester]

I cant fit screen shots of before, the stack traces are very long.

Can you just show part of the screen? It would help to have a record of what it looks like in terms of before and after.

[weavejester]

Also in terms of review, please see the contributing guidelines first. There's a bunch of formatting changes that need to be taken out, and the commit messages need to adhere to the guidelines.

[sirmspencer]

Also in terms of review, please see the contributing guidelines first. There's a bunch of formatting changes that need to be taken out, and the commit messages need to adhere to the guidelines.

I read the guidelines and did as best as I can interpret on the commit messages, and they are updated from the initial push. You have to realize that I have never had a project with those types of guidelines, it's not obvious to me what you are asking me to change.

As far as the code, I dont see any formatting changes in the PR. Please post a review on github, I can make any changes you ask for.

[sirmspencer]

Thank you for the review that helped me understand what you are looking for. I found a couple more formatting changes to remove after making the updates you pointed out. Sorry, I didnt see the clojure guide also had a line length. Not fixing indentation on the code I'm working on is new to me. Should I create a PR after this one to fix the indents, and other formatting?

[weavejester]

Another PR to fix indentation would be fine. The reason we don't mix formatting changes with feature changes is that it muddies the diff and makes it hard to understand what has changed. If we separate out formatting changes from other changes, it's a lot easier to see which lines have changed.

This code looks like it could be written more concisely; there are a lot of functions where I think a refactor would significantly reduce the changes required. I've also briefly looked into :trimmed-elems and I don't understand why it would affect things that aren't a cause. I'm going to need to dedicate more time to this, but it's going to take at least a few weeks before I can manage that.

[sirmspencer]

This code looks like it could be written more concisely; there are a lot of functions where I think a refactor would significantly reduce the changes required. I've also briefly looked into :trimmed-elems and I don't understand why it would affect things that aren't a cause. I'm going to need to dedicate more time to this, but it's going to take at least a few weeks before I can manage that.

You are correct that the root does not have trimmed elements, only causes do. The root :trace-elems are not very helpful because it returns the same 40+ stack trace of the application for every error, with no error details. So the function does end up setting :trace-elems to empty for the root. You can see in the screen shots, that the remaining information gets right to the point of the error.

[weavejester]

Would it be better if we just made certain segments of the stacktrace collapsable? I'm also wondering if we want to test this out in an external library first, as I currently lack the time to investigate this thoroughy.

[sirmspencer]

Would it be better if we just made certain segments of the stacktrace collapsable? I'm also wondering if we want to test this out in an external library first, as I currently lack the time to investigate this thoroughy.

I could do this, instead of 1 new option, it would be two: collapse-root? and trimmed-cause? The code would get more complex though. As written works for me, until someone requests some parts collapsible and other not, it feels like extra.

I have the code running in my current project from a local NS. How would I get it into an external library for testing?

[weavejester]

You can just pull the namespace out, give it a new name, and release it as part of a new library. That will solve the problem in the short term.

In the longer term I'll get around to looking at this eventually, but I'll need to budget several days review. Currently I don't feel as though I understand the consequences of this change well enough to even begin to think about merging it, and I'm pretty sure we could make the code more concise in places.

[sirmspencer]

You can just pull the namespace out, give it a new name, and release it as part of a new library. That will solve the problem in the short term.

In the longer term I'll get around to looking at this eventually, but I'll need to budget several days review. Currently I don't feel as though I understand the consequences of this change well enough to even begin to think about merging it, and I'm pretty sure we could make the code more concise in places.

Thanks for all your feedback. To help the review process, I can break out the changes for formatting, color? and trimmed?.

[sirmspencer]

This PR will be split, and is no longer needed.