.NET C# stacktrace support #5489 #5534
Conversation
Fixes: Issue eclipse-che#5489 Signed-off-by: Victor Rubezhny <vrubezhny@redhat.com>
|
Can one of the admins verify this patch? |
|
ci-build |
benoitf
added
kind/enhancement
|
Build # 2994 - FAILED Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/2994/ to view the results. |
|
ci-build |
|
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/2996/ |
| * installs the handler functions for the links. The handler parses the stack | ||
| * trace line, searches for a candidate C# file to navigate to, opens the found | ||
| * file in editor and reveals it to the required line and column (if available) | ||
| * according to the line information |
There was a problem hiding this comment.
you should add the @author tag
There was a problem hiding this comment.
Next time is OK?
Or I should add it to all the output customizer like classes (as well as to those which was added only in fix for CHE-15)?
There was a problem hiding this comment.
I approved the PR so it's up to you
| if (lineOffsetStart == -1) { | ||
| lineOffsetStart = 0; | ||
| } | ||
| static void openFileInEditorAndReveal(AppContext appContext, EditorAgent editorAgent, Path file, |
There was a problem hiding this comment.
should we set an accessor ?
There was a problem hiding this comment.
I've meant to make it to be package package available this time. Can't imagine a class that may be in need of calling this method other than these two *OutputCustomizer-classes at the moment.
If output customizers will be distributed among the different packages in future, we'll have to make it to be like public API. However at the moment, we have all these classes placed closely in one package and we don't have a mean that let adopters to add their own customizers - so no need to make it like public API.
IMHO
There was a problem hiding this comment.
I was thinking private but it was more a thought. No change requested.
There was a problem hiding this comment.
No, it can't be private - it's used from two different classes in the same package.
There was a problem hiding this comment.
my feeling was that DefaultOutputCustomizer is not a default one, it's including Java or JavaScript.
So static code should be moved in a Empty/base or default class
But we should have Java(Script)OutputCustomizer and CSharpOutputCustomizer (they could both extend the same class)
There was a problem hiding this comment.
@benoitf I have the same feeling. Default* was like a first impression (and because java is most used language, imho) - so, with having two more customizers [at the moment] it doesn't look like a proper name for this kind of customizer.
So, shell I turn DefaultOutputCustomizer into an abstract class (with few utility methods, like openFileInEditorAndReveal(...), selectRange(...) and set of abstract methods of OutputCustomizer interface) and a class like JavaOutputCustomizer (with realization for Java/JavaScript stacktraces)? (before the applying of this PR I mean)
There was a problem hiding this comment.
@vrubezhny I would say that it may be clearer this way, yes
There was a problem hiding this comment.
@benoitf @slemeur I've pushed a fixup commit that separates DefaultOutputCustomizer into AbstractOutputCustomizer (with some commonly used utility methods) and JavaOutputCustomizer. All the existing customizers are adopted to this change.
The similar thing is made for JUnit tests: DefaultOutputCustomizerTest is separated into BaseOutputCustomizerTest (contains commonly used utility methods) and JavaOutputCustomizerTest. All the existing output customizer tests are adopted to this change as well.
There was a problem hiding this comment.
@vrubezhny yes it's definitely better. Thanks.
Signed-off-by: Victor Rubezhny <vrubezhny@redhat.com>
Signed-off-by: Victor Rubezhny <vrubezhny@redhat.com>
Signed-off-by: Victor Rubezhny <vrubezhny@redhat.com>
Signed-off-by: Victor Rubezhny <vrubezhny@redhat.com>
|
ci-build |
|
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/3021/ |
|
@vparfonov @benoitf if we're ok with merging it, let's do it. Maybe a full QA cycle is required? |
benoitf
removed
the
status/code-review
* .NET C# stacktrace support eclipse-che#5489 Fixes: Issue eclipse-che#5489 Signed-off-by: Victor Rubezhny <vrubezhny@redhat.com>
Adds the possibility to click on a stacktrace and compilation error/warning message lines in order to open a specified .NET C# class in editor.
Example:
Fixes #5489
Fixes #5546
Signed-off-by: Victor Rubezhny vrubezhny@redhat.com
What does this PR do?
Adds the possibility to click on a stacktrace and compilation error/warning message lines in order to open a specified .NET C# class in editor.
What issues does this PR fix or reference?
Issue #5489
Issue #5546
Changelog
Release Notes
This version introduces new capabilities to the .NET and C# developers. Eclipse Che, now analyzes stacktraces and compilation error/warning messages from the console to intercept files and lines references. Those references are highlighted as links, when clicked, it opens .NET C# class in editor and reveal it to the line according to the info provided in the console output line.
[Animated Gif]
The smart selection capability is available for .NET C# stacktraces and compilation error/warning messages as well as for Java and NodeJS stacktraces at this moment.
Docs PR