-
Notifications
You must be signed in to change notification settings - Fork 874
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
Stacktrace analyzer window fixes #6841
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
recommend to enable ignore whitespace changes for reviewing this one - sorry about this
there you go, found the evil logger: see #6843 and apache/maven#1342 |
7535f37
to
3101fde
Compare
3101fde
to
8cd2066
Compare
private static final Pattern LINE_PATTERN = Pattern.compile( | ||
"at\\s" + // initial at // NOI18N | ||
"at\\s+" + // initial at // NOI18N | ||
"("+IDENTIFIER+"(?:\\."+IDENTIFIER+")*/)?" + // optional module name // NOI18N | ||
"(("+IDENTIFIER+"(\\."+IDENTIFIER+")*)\\.)?("+IDENTIFIER+")" + // class name // NOI18N | ||
"\\.("+IDENTIFIER+"|\\<init\\>|\\<clinit\\>)\\((?:"+IDENTIFIER+"(?:\\."+IDENTIFIER+")*/)?" +IDENTIFIER+"\\.?("+IDENTIFIER+ // method and file name // NOI18N | ||
")\\:([0-9]*)\\)"); // line number // NOI18N | ||
"\\.("+IDENTIFIER+"|\\<init\\>|\\<clinit\\>)\\s*"+ // method name | ||
"\\((?:"+IDENTIFIER+"(?:\\."+IDENTIFIER+")*/)?"+IDENTIFIER+"(\\.(?:\\p{javaJavaIdentifierPart}+))?"+ // '(File.java' // NOI18N | ||
"\\:([0-9]*)\\)"); // ':123)' // NOI18N |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the original goal was to add a few optional \\s*
but this does also include a fix for https://github.com/apache/netbeans/pull/6640/files#r1377502665, since this didn't work before as pointed out by @dbalek in the linked review.
The file extension group is now optional (\\.(?:\\p{javaJavaIdentifierPart}+))?
and matches more character types since extensions aren't real java identifiers. Tests cover this now too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks sane to me. I left two inline comments, but these are nitpicks.
java/java.navigation/src/org/netbeans/modules/java/stackanalyzer/StackLineAnalyser.java
Outdated
Show resolved
Hide resolved
java/java.navigation/src/org/netbeans/modules/java/stackanalyzer/StackLineAnalyser.java
Outdated
Show resolved
Hide resolved
- update matcher to be more forgiving when traces are pasted from ANSI decorated sources (some loggers use colors for exception elements) - some loggers add extra spaces to stack trace lines - properly escape angle brackets in cell renderer - some indentation fixes for lines which are rendered without link (e.g native code calls) - avoid using deprecated methods
8cd2066
to
5fc30b2
Compare
(e.g native code calls)
example: this trace isn't parsed properly NB 20 (note: the space before '('):
even if the space is manually removed the result could be better (missing

<init>
, broken indentation on at-lines which can't be linked):with this PR applied:
