Skip to content
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

Allow customzing css classes for the ErrorsViewStackTracePrinter #13686

Merged
merged 2 commits into from
Oct 1, 2024

Conversation

codeconsole
Copy link
Contributor

<g:renderException exception="${exception}" detailsClass="alert alert-danger" stackClass="bg-body-secondary" snippetClass="bg-body-secondary snippet" lineErrorClass="bg-danger" />

Copy link
Contributor

@matrei matrei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want to add features to the 6.2.x branch?
I'm not sure "this is the way".

@@ -23,7 +23,7 @@ import org.codehaus.groovy.control.MultipleCompilationErrorsException
*/
class DefaultStackTracePrinter implements StackTracePrinter {

String prettyPrint(Throwable t) {
String prettyPrint(Throwable t, Map attrs = [:]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using Groovy default params on overridden/implemented methods makes the code hard to follow. Can we use:

@Override
String prettyPrint(Throwable t) {
    prettyPrint(t, [:])
}

String prettyPrint(Throwable t, Map attrs) {
    
}

(or change the interface to include both versions)

@@ -47,7 +47,7 @@ class DefaultErrorsPrinter extends DefaultStackTracePrinter implements CodeSnipp
res == null ? te.className : res.getFilename()
}

String prettyPrintCodeSnippet(Throwable exception) {
String prettyPrintCodeSnippet(Throwable exception, Map attrs = [:]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using Groovy default params on overridden/implemented methods makes the code hard to follow. Can we use:

@Override
String prettyPrintCodeSnippet(Throwable t) {
    prettyPrintCodeSnippet(t, [:])
}

String prettyPrintCodeSnippet(Throwable t, Map attrs) {
    
}

(or change the interface to include both versions)

@codeconsole
Copy link
Contributor Author

You want to add features to the 6.2.x branch? I'm not sure "this is the way".

Norgifn breaking here. Does overloading a method really warrant going from 6.2.0 to 6.3.0 if the previous method still exits and works the same as before?

@matrei
Copy link
Contributor

matrei commented Sep 25, 2024

Norgifn breaking here.

You don't know that until it's too late.
Why is this important to get this into Grails 6?

@codeconsole
Copy link
Contributor Author

You don't know that until it's too late. Why is this important to get this into Grails 6?

It's not critical, but I am trying to overhaul the css in Grails 7 and the only efficient way of me doing it is against live code in Grails 6. I am trying to stay productive and utilize my time as efficiently as possible.

All we are doing here is allowing the default css classes to be substituted. Otherwise the same behavior is persisted with no changes.

This is easily tested by just throwing an exception and viewing the error page which I currently am doing locally.

@codeconsole codeconsole merged commit 8079fa5 into apache:6.2.x Oct 1, 2024
12 checks passed
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.

None yet

2 participants