Community
Participate
Working Groups
Java is notorious for it's long stack traces, so it would be nice, if Eclipse could help understanding what types are code touched during debuging. Currently every stack frame in the debug view has the same color, I would like to have different colors for: * platform frames (for java.*, javax.*, org.junit.*, org.eclipse.jdt.*) * frames coming from libraries * user code (maybe separate color for user codes sitting in a test source folder) * any code, which filtered out with the step filtering setup.
This is related a bit to - https://bugs.eclipse.org/bugs/show_bug.cgi?id=548967 and to: https://bugs.eclipse.org/bugs/show_bug.cgi?id=111596
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/181896
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/181897
Thanks Zsombor. Will look at this next week.
I've squashed the two commit together, hopefully, it's still understandable. There are a couple of missing thing, which I wanted to achieve, but unable to figure out, how to do it: * I could detect if the stack frame originated from a class, which is stored in a jar file , or it's a proper '.class' file. Based on this, I could categorize as library code or project code. However, I couldn't find a way to match that java.io.File with a project, and compilation target folder. What is the proper/simple way to do it? * As the step filtering is not happening inside Eclipse, but on the debugged JVM - which totally makes sense in hindsight - I can't color the stack frames based on the user provided 'step filters'. Doing a simple, new implementation in the UI wouldn't be a big issue, to handle package prefixes, but if you know a better way to do it, please tell me! * I defined the 'platform' code as anything, what I see as a noise in the stack traces, when I run an app, or run a test, so I've hard-coded as classes from "java., javax., jdk., com.sun, org.junit., org.eclipse.jdt.internal.". So internal JVM classes, and all the support code in JUnit and in JUnit runner is grayed out now. What do you think, is it a good solution? * All the stack frame categorization happens in the UI code, and the same categorization runs every time - and for each stack frame twice, once for the foreground color, and once for the background color. In my initial testing, it hasn't added any noticeable slowness, but maybe, this could be an issue. Maybe, the result of the categorization could be stored inside the IJavaStackFrame objects(interfaces) directly, what do you think? It would certainly complicates the life of the stack frame objects, but maybe, it would be a more efficient runtime solution. Thanks for having a look!
Got delayed in the review. Some feedback : 1. we cannot hardcode the colours for categories, we can give some default values but used needs to be able to change it. 2. While calculating the Foreground calculate the background also and the tricky part is to store and then remove after used once ? Need to think more here. 3. List of PLARFORM_PACKAGES can also be defined by user apart from some default added by system 4. Can we support adding of new categories ? this can be used insted of Step filters.
(In reply to Sarika Sinha from comment #6) > Got delayed in the review. Some feedback : > 1. we cannot hardcode the colours for categories, we can give some default > values but used needs to be able to change it. Yes, this is why I did this: https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/181897/5/org.eclipse.jdt.debug.ui/plugin.xml It seems to be working - apart re-coloring the frames only happen when the stack frame is changed - so not when the 'Close' or 'Apply' button is pressed on the preference dialog. I haven't found a simple way to wait for color changes and re-triggering the rendering of the Debug view. > 2. While calculating the Foreground calculate the background also and the > tricky part is to store and then remove after used once ? Need to think more > here. Is there anything against putting it into org.eclipse.jdt.debug.core.IJavaStackFrame/org.eclipse.jdt.internal.debug.core.model.JDIStackFrame directly? Why do you want to remove the categorization after it is used for foreground/background/font/etc ? Isn't the JDIStackFrames are re-created every time, when a user has a breakpoint? > 3. List of PLARFORM_PACKAGES can also be defined by user apart from some > default added by system Yes, some customization is a must, I think, how complicated do we want to have it. I guess, we can 'copy' the Step filter configuration dialog. > 4. Can we support adding of new categories ? this can be used insted of Step > filters. I don't know: the downsides are that instead of having a fixed enum to cover all the categories, we would need to create some sort of ids to identify the category, and instead of the declarative colorDefinition tags in the plugin.xml, we would need to declare the categories dynamically - or we would need to provide a custom color selector component. Do you have any use case, where defining more categories would make sense?
(In reply to Zsombor from comment #7) > > Yes, this is why I did this: > https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/181897/5/org.eclipse.jdt. > debug.ui/plugin.xml > It seems to be working - apart re-coloring the frames only happen when the > stack frame is changed - so not when the 'Close' or 'Apply' button is > pressed on the preference dialog. I haven't found a simple way to wait for > color changes and re-triggering the rendering of the Debug view. It is ok to have the expectation of getting the preference effective only for the next Debug session. We do that for many Editor properties as well.It will not change the current debug view but new debug view can take the new pref values. > > Is there anything against putting it into > org.eclipse.jdt.debug.core.IJavaStackFrame/org.eclipse.jdt.internal.debug. > core.model.JDIStackFrame directly? Why do you want to remove the > categorization after it is used for foreground/background/font/etc ? Isn't > the JDIStackFrames are re-created every time, when a user has a breakpoint? Yes I don't foresee much of an issue. > Yes, some customization is a must, I think, how complicated do we want to > have it. I guess, we can 'copy' the Step filter configuration dialog. > > > I don't know: the downsides are that instead of having a fixed enum to cover > all the categories, we would need to create some sort of ids to identify the > category, and instead of the declarative colorDefinition tags in the > plugin.xml, we would need to declare the categories dynamically - or we > would need to provide a custom color selector component. Do you have any use > case, where defining more categories would make sense? What I had in mind was like I want to ignore or pay extra attention to a particular package in the stackframe while debugging and I assign a different colour, then it helps me to quickly navigate to it.
The patch has grown a bit, as I've added a preference page to manage filters. As these filters functionality are very similar to the step filter preferences, I've refactored the code a bit, so I could reuse it. Not all the dialogs/labels are correct yet, but currently, it looks functional. What do you think?
(In reply to Zsombor from comment #9) > The patch has grown a bit, as I've added a preference page to manage > filters. As these filters functionality are very similar to the step filter > preferences, I've refactored the code a bit, so I could reuse it. Not all > the dialogs/labels are correct yet, but currently, it looks functional. What > do you think? Will have a look.
Patch seems to be in the right direction. Only thing missing I see is the ability to know the information about the categories and colours. In the preference page if the categories can be listed and a colour picker can be added, it will help. As many people cannot see all the colors equally and specially in dark theme. I am ok to add the colour picker in the follow up gerrit for RC1 but we need to highlight the categories and the current colours for now. Testcases needs to be added. If you can add these 2 things, we can release first version in M2 so that it is tested more widely.
Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/181897 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=c2dd7af8f0c9161182a157e0768d5bb5feed5c77
Zsombor, Sarika: can we have some N&N with screenshots / explanation of the feature?
Thank you Zsombor for working on the suggestions quickly. I have released the changes as today is the last day for M3 contribution. We need 2 more things to be done - 1. Restore Defaults should restore the colour selection to defaults. 2. Add N&N entry to JDT Debug section in news repo www.eclipse.org/eclipse/news.git
(In reply to Sarika Sinha from comment #14) > We need 2 more things to be done - > 1. Restore Defaults should restore the colour selection to defaults. > 2. Add N&N entry to JDT Debug section in news repo > www.eclipse.org/eclipse/news.git May be also: 3. Reconsider to enable this feature in 4.22 and fine tune the implementation / colors in 4.22? Default coloring schema seem to be a bit odd / distracting. I can hardly see stack traces with light gray foreground color, also "test" coloring seem not to work at all. Neither "Production code" nor "Library methods" coloring seem to be defined in UI (and without looking into the code I have no idea how that should work). Would it work, I would probably dislike the heavy green background color anyway. Also I can't understand why do we mix step filters and coloring in the top message in the preferences, as if the "step filters" only there for coloring. I would move everything that belongs to "Stack colors" to a dedicated group below the actual filter section. I personally would revert that for 4.21, as of now this feature looks "not done" for me.
Created attachment 286953 [details] My debug experience is not nice I would not like to debug with *that* setup, see screenshot. Preferences are not well defined either.
Sarika, can we postpone introduction of this feature for 4.22? Last day of M3 is a bit too late for such changes. Either disable by default or revert?
Colours can be changed by users . We can change the default colours as well. We need Help to specify what each category means. Other option is to switch this off by default and keep for 4.21, WDYT ? I am open to revert and put in 4.22 also.
(In reply to Sarika Sinha from comment #18) > Colours can be changed by users . > We can change the default colours as well. > We need Help to specify what each category means. > > Other option is to switch this off by default and keep for 4.21, WDYT ? > > I am open to revert and put in 4.22 also. Looking further in the code I see other issues. Small one: StackFramePresentationProviderTest is not added to the suite. Bigger one: org.eclipse.jdt.internal.debug.ui.StackFramePresentationProvider.categorizeDirectory(File) iterates over every project and every source classpath entry to colorize stack if the code is inside directory. That can't scale with lot of projects / long stacks. Small: Boolean.parseBoolean(ClasspathEntry.getExtraAttribute(classPathEntry, "test")) can be replaced with IClasspathEntry.isTest(). I also not sure how that scales in general, e.g. for every stack element that is not system/synthetic we ask *twice* if that class is a directory or not (which is IO operation). It is not clear to me if the colors can be changed by other applications based on Java debugger. E.g. how to override default colors/filters etc - only via product customization, or can it be done dynamically? The new preference page "Stack Frames" is confusing me also. I believe it should be named "Stack Appearance", and internally it must be made more clear what is what and how all the categories are defined etc. I would vote for revert and another review round in 4.22. Simeon, WDYT?
Thinking a bit more about UI of the preference page, I think all hard coded (and unclear to user) categories should be removed, current UI reworked completely. Similar to "Detail Formatters" we should have a freely modifiable table on top with categories. Every selected category in the table would allow to define (in the extra table below) patterns, required flags as checkboxes (e.g. source code / test code / library code / synthetic code etc) and FG/BG color. By default we could propose few categories but user could define as many as they like.
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/183773
Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/183773 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=40f4910452429171bcad6d7f33615f0204a4d84a
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/183774
(In reply to Eclipse Genie from comment #22) > Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/183773 was > merged to [master]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/ > ?id=40f4910452429171bcad6d7f33615f0204a4d84a Sarika, we can't revert change of the bundle version, it must be bumped again, otherwise Maven will use old bundle.
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/184089
Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/184089 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=14ccbcea70acbd8dfe9fef53140df158fc5060db
(In reply to Andrey Loskutov from comment #19) > (In reply to Sarika Sinha from comment #18) > > Colours can be changed by users . > > We can change the default colours as well. > > We need Help to specify what each category means. > > > > Other option is to switch this off by default and keep for 4.21, WDYT ? > > > > I am open to revert and put in 4.22 also. > > Looking further in the code I see other issues. > > Small one: StackFramePresentationProviderTest is not added to the suite. > Will have a look on it. > Bigger one: > org.eclipse.jdt.internal.debug.ui.StackFramePresentationProvider. > categorizeDirectory(File) iterates over every project and every source > classpath entry to colorize stack if the code is inside directory. That > can't scale with lot of projects / long stacks. > > Small: Boolean.parseBoolean(ClasspathEntry.getExtraAttribute(classPathEntry, > "test")) can be replaced with IClasspathEntry.isTest(). > > I also not sure how that scales in general, e.g. for every stack element > that is not system/synthetic we ask *twice* if that class is a directory or > not (which is IO operation). We don't ask twice, the categorization is stored inside the stack frame. Anyway, I've found a different solution to differentiate between test and prod code, however it needs a change in the jdt.core repo too. > > It is not clear to me if the colors can be changed by other applications > based on Java debugger. E.g. how to override default colors/filters etc - > only via product customization, or can it be done dynamically? > > The new preference page "Stack Frames" is confusing me also. I believe it > should be named "Stack Appearance", and internally it must be made more > clear what is what and how all the categories are defined etc. > The colors can be configured on the regular color preference pages, maybe adding some kind of a link to navigate there would be nice. The descriptions are certainly needing improvements. > I would vote for revert and another review round in 4.22. > > Simeon, WDYT?
(In reply to Andrey Loskutov from comment #20) > Thinking a bit more about UI of the preference page, I think all hard coded > (and unclear to user) categories should be removed, current UI reworked > completely. > > Similar to "Detail Formatters" we should have a freely modifiable table on > top with categories. Every selected category in the table would allow to > define (in the extra table below) patterns, required flags as checkboxes > (e.g. source code / test code / library code / synthetic code etc) and FG/BG > color. > > By default we could propose few categories but user could define as many as > they like. There are a couple of problems what I see with to much customization: * currently the categorization could be stored as an self-explaining enum, making a lot of things simpler * if there are arbitrary number of categories, it makes the integration with the color management infrastructure a bit harder, which currently needs the color declarations in a static xml file. This would make the 'dark theme' support also more problematic (the current implementation still look a bit ugly on dark theme, which we need to fix) * defining a category could become very complicated, because at least we need to specify: ** package filter ** if it's a synthetic method ** if the source code from the test-classpath ** if the source code from a non-test-classpath ** if the code from a jar ** if the code from the Java Runtime Certain combinations of these requirements would make sense - package filter AND restrictions if source code is test or not - certain wont like 'synthetic method' AND 'code from a jar', presenting this in an understandable way would be challenging.
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/185329
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/185330
Zsombor, i like the general idea. Thanks for working on it. I don not see a need for so many categories. I fear the autocomputed categories can influence performance when many stack elements (many threads!) are colored (needs to be tested). I would prefer only 3 colors/categories: 1) normal, 2) greyed-out, 3) hidden, only 2) would need a color and that should be a fixed readable grey value. 3) should be just skipped. Both 2) and 3) could be fully configured by classpath. by default 3) should be empty and 2) should contain also typical DI libraries like Google Guice Given such simple defaults i would even like the feature be enabled by default. If you have a strong reason to have more categories i would like them all to be configured in the same way with the following defaults: 4) highlighted 1 (*BOLD*) 5) highlighted 2 (light red background) 6) highlighted 3 (light yellow background)
As each JDIStackFrame is evaluated only once, I really hope, the categorization wont be slow, even for deep stack traces. In my experience using this patch in the recent months, most of the stack trace element covered by platform/custom filters - so for org.eclipse.* or org.spring.* - or automatically generated, proxy or lambda call is categorized without any classpath lookups. I thought about hiding stack frames from the view - but apart from complicates the implementation a bit - I fear, it would confuse the users, for example: * if suddenly they just step into a function, but couldn't see them in the thread view * or they just couldn't see the expected transaction handling code from Spring, etc ... Maybe, it's possible to add some sort of '*** (3 stack frame are hidden) ***' marker in the view, which could expand when the user double click on it. But that implementation would be beyond my current limited SWT knowledge. Is there any documentation, guideline about color usage in Eclipse? I haven't found anything unfortunately. Sorry, for the not-so-nice default colors which you can see in the patch.
If you do not find intuitive colors, you may consider to keep black on white but exchanging the symbols in front. In the Package Explorer there are different symbols for System Library, .jar and sources etc.. you could try those. Maybe it would be intuitive to see them in Stacktraces too. Maybe with an overlay symbol of the blue stacktrace bars.
It will be good if we can decide on this feature and release it in 4.22.
@Zsombor, I see that you have uploaded a new patch to https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/185330 Can you highlight what are the changes? What is done and what is not done? Are you planning to add more changes? This will help in reviewing.
Sorry for the long silence, and lack of messaging. Since September I've created a separate component for editing filters - in https://bugs.eclipse.org/bugs/show_bug.cgi?id=576615 - as it is not yet reviewed/merged the gerrit build fails, as this patch depends on that code from eclipse.jdt.ui. Apart from this: * I've made each category independently switchable on or off * Tried to make the colors less harsh and a bit softer, and adjusted the labels a bit. * Added custom icons for the prod/test category - it looks better, than I initially thought it would look like. Currently I'm using it, and generally happy with the results.
Created attachment 287826 [details] current screenshot
Thanks, can you attach the screenshot from the new filter preference page also to indicate switching off etc of the categories?
Created attachment 287844 [details] preference page Here is the latest preference page
(In reply to Eclipse Genie from comment #30) > New Gerrit change created: > https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/185330 This gerrit fails to compile, It is using some classes which are not part of Build : import org.eclipse.jdt.ui.filter.FilterManager; import org.eclipse.jdt.ui.filter.JavaFilterTable; import org.eclipse.jdt.ui.filter.JavaFilterTable.ButtonLabel; import org.eclipse.jdt.ui.filter.JavaFilterTable.DialogLabels; import org.eclipse.jdt.ui.filter.JavaFilterTable.FilterTableConfig; Do you plan to push one patch for JDT UI with these classes?
(In reply to Sarika Sinha from comment #40) > (In reply to Eclipse Genie from comment #30) > > New Gerrit change created: > > https://git.eclipse.org/r/c/jdt/eclipse.jdt.debug/+/185330 > > This gerrit fails to compile, It is using some classes which are not part of > Build : > import org.eclipse.jdt.ui.filter.FilterManager; > import org.eclipse.jdt.ui.filter.JavaFilterTable; > import org.eclipse.jdt.ui.filter.JavaFilterTable.ButtonLabel; > import org.eclipse.jdt.ui.filter.JavaFilterTable.DialogLabels; > import org.eclipse.jdt.ui.filter.JavaFilterTable.FilterTableConfig; > > Do you plan to push one patch for JDT UI with these classes? Yes, I've already pushed the change more than a month ago: https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/186460
Thanks!I have asked for the review of JDT UI code. Few points from the current Debug gerrit: 1. Preference page does not look organized. All the check boxes should be together. The colour selection section can be moved up. The adding custom packages section can be pushed down and can be a little smaller size. This will help most of the people to avoid scrolling the page down. 2. What is "Production Code" ? (Sorry I have not checked the code to understand) 3. Which category take precedence when multiple are selected is not obvious to the user. @Andrey, You want to add some?
(In reply to Sarika Sinha from comment #42) > Thanks!I have asked for the review of JDT UI code. > > Few points from the current Debug gerrit: > 1. Preference page does not look organized. All the check boxes should be > together. The colour selection section can be moved up. The adding custom > packages section can be pushed down and can be a little smaller size. This > will help most of the people to avoid scrolling the page down. > 2. What is "Production Code" ? (Sorry I have not checked the code to > understand) > 3. Which category take precedence when multiple are selected is not obvious > to the user. > > @Andrey, > You want to add some? Thanks for the review! Yes, that preference page is far from perfect. My idea behind the bit dis-organized look, to follow the actual logic for picking a color. 1, First - the 'Custom' filter - maybe in the future, we could even add an action for classes/stack frames, to 'Mark as custom/highlighted frame'. So this should be the most distinct. 2, As there is no much reason to look into syntethic methods, because there is no source code or debug information available regularly, I thought it's better to mark as less relevant early. 3, Platform code - for the classes coming from the JDK, JUnit and Eclipse test launcher, they are relatively well defined, and rarely interesting methods. 4, Test code - any source code which live in a source folder marked as "Test" 5, Production code - yes, it's not the best wording, the most accurate description would be: "User written source code, from a source folder, which doesn't marked as "Test"". What could be a better name? "Own code"? "My non-test code"? "My source code"? To better explain the concept and the logic behind, what UI component should I use? Maybe, a tree would make it nicer?
Production can be renamed to "Source" ? Any other suggestion is also welcome. Not able to visualize how will it look with tree, if you can make a small mockup (not detailed) it can get us some idea.
(In reply to Sarika Sinha from comment #44) > Production can be renamed to "Source" ? > Any other suggestion is also welcome. > > Not able to visualize how will it look with tree, if you can make a small > mockup (not detailed) it can get us some idea. ping!
(In reply to Sarika Sinha from comment #45) > (In reply to Sarika Sinha from comment #44) > > Production can be renamed to "Source" ? > > Any other suggestion is also welcome. > > > > Not able to visualize how will it look with tree, if you can make a small > > mockup (not detailed) it can get us some idea. > > ping! Sorry, totally forgot this ticket. I've updated the patch to the latest version numbers, and modified the labels to: 'Your Source code' and 'Your Test code' and instead of 'Custom filter...' I came up with the 'Highlighted stack frames', to better explain the idea, why that would be useful. Does it make sense to you? Maybe, we could add an action to IJavaStackFrames, which quickly highlight methods from that class? I can reorder the components as you previously suggested, I don't think I can come up better UI :(
(In reply to Zsombor from comment #46) > (In reply to Sarika Sinha from comment #45) > > (In reply to Sarika Sinha from comment #44) > > > Production can be renamed to "Source" ? > > > Any other suggestion is also welcome. > > > > > > Not able to visualize how will it look with tree, if you can make a small > > > mockup (not detailed) it can get us some idea. > > > > ping! > > Sorry, totally forgot this ticket. > I've updated the patch to the latest version numbers, and modified the > labels to: > 'Your Source code' and 'Your Test code' and instead of 'Custom filter...' I > came up with the 'Highlighted stack frames', to better explain the idea, why > that would be useful. Does it make sense to you? Maybe, we could add an > action to IJavaStackFrames, which quickly highlight methods from that class? > I can reorder the components as you previously suggested, I don't think I > can come up better UI :( Looks much better now. Can we just group the check boxes which do not have dependent actions together? May be rename the "Mark Stack Frames differently" check box as something to start with Enable so that it is clear. You can see Code Minings Preference page for reference. The 2 filter can be made to occupy a little less space.
@Zsombor, Can you convert this to PR in github at https://github.com/eclipse-jdt/eclipse.jdt.debug
