Community
Participate
Working Groups
From a cdt-dev discussion at: http://dev.eclipse.org/mhonarc/lists/cdt-dev/msg19960.html Marc Khouzam wrote: "We've had complaints that the build console is only opened by the first build. For example, bug 297222 "Allow temporary build arguments from build console" would allow to add arguments for the build directly from the build console, but to see the console, we'd need to do a build first :-( " Andrew Gvozdev replied: "There is a dropdown box on the right of the view listing all available consoles and you can select one. I think we should add CDT build console there. " I like that suggestion a lot. It would solve the issue the way Eclipse has addressed this before.
Created attachment 175340 [details] Attempt at a fix Here's is an attempt at providing this functionality. The soltuion is actually very simple and based on my tests, it works well. I'm just wondering if it is safe to open the build console before triggering a build. Is there something that is initialized when we trigger a build, which won't be in this case?
I opened up eclipse and went straight to the console. I am getting exceptions like that trying to use any console controls: org.eclipse.core.runtime.AssertionFailedException: null argument: at org.eclipse.core.runtime.Assert.isNotNull(Assert.java:85) at org.eclipse.core.runtime.Assert.isNotNull(Assert.java:73) at org.eclipse.cdt.internal.ui.buildconsole.BuildConsoleManager.getConsole(BuildConsoleManager.java:317) at org.eclipse.cdt.internal.ui.buildconsole.BuildConsolePage.moveToError(BuildConsolePage.java:558) at org.eclipse.cdt.internal.ui.buildconsole.PreviousErrorAction.run(PreviousErrorAction.java:39) at org.eclipse.jface.action.Action.runWithEvent(Action.java:498) at org.eclipse.jface.action.ActionContributionItem.handleWidgetSelection(ActionContributionItem.java:584) at org.eclipse.jface.action.ActionContributionItem.access$2(ActionContributionItem.java:501) at org.eclipse.jface.action.ActionContributionItem$6.handleEvent(ActionContributionItem.java:452) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1053) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4066) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3657) at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2629) at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2593) at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2427) at org.eclipse.ui.internal.Workbench$7.run(Workbench.java:670) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:663) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:115) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:369) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at java.lang.reflect.Method.invoke(Unknown Source) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:619) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:574) at org.eclipse.equinox.launcher.Main.run(Main.java:1407) at org.eclipse.equinox.launcher.Main.main(Main.java:1383)
(In reply to comment #2) > I opened up eclipse and went straight to the console. I am getting exceptions > like that trying to use any console controls: Thanks Andrew. It was sloppy of me not to try each control. I see the exceptions as well. I'll see if it is easy to fix.
Created attachment 175440 [details] Fix including fix for "Copy build log" action From what I can see, there are two problem cases: 1- next error/previous error 2- Copy build log These actions don't like it when no project was ever selected. It is easy enough to put a check for the project to not be null, but I was wondering if we shouldn't go a little further. Should the above actions really be enabled when they won't work? If there is not error or warning in the console, maybe we should disable the next/previous error actions. As for the Copy build log, the action works even when the console is empty, but causes a problem when no project is selected. So, for that I think we should just put a check for project != null. The new patch fixed #2. I'll wait for your opinion before fixing #1
Created attachment 175452 [details] Better fix (In reply to comment #4) > Created an attachment (id=175440) > Fix including fix for "Copy build log" action > > From what I can see, there are two problem cases: > > 1- next error/previous error > 2- Copy build log There was a third problem, when selecting the ShowErrorInEditor toggle. > These actions don't like it when no project was ever selected. It is easy > enough to put a check for the project to not be null, but I was wondering if we > shouldn't go a little further. > > Should the above actions really be enabled when they won't work? If there is > not error or warning in the console, maybe we should disable the next/previous > error actions. Disabling these actions did not seem simple. Getting the buildPartitioner to notify the BuildConsolePage that errors/warning extisted or not did not seem easy to me. Therefore, I figure that keeping the behaviour as is, which is to have those actions do nothing when there are no errors in the console, would be a good enough solution. The last patch does this. > As for the Copy build log, the action works even when the console is empty, but > causes a problem when no project is selected. So, for that I think we should > just put a check for project != null. I actually thought this would be a little cofusing to the user, to have the action do nothing, when it used to always work. Therefore I added and error dialog which tells the user that a build must be done before trying to copy its log. I believe this patch resolves any assertion exceptions or NPEs that were caused by using actions from the build console when no project had been selected or built.
(In reply to comment #5) > Created an attachment (id=175452) > Better fix Thanks, don't see the exceptions now. Here are some more comments: - The message popping up on "Copy Build Log" is not accurate. It says "A project must be built to be able to save its build log." The actual problem is that no project is selected. I'd rather not to have any popup at all here. One could hardly expect to copy any log in this case. But one could expect the action to be disabled, that's true. > Should the above actions really be enabled when they won't work? If there is not error or warning in the console, maybe we should disable the next/previous error actions. - I think it should be possible to disable "Copy Build Log". What it does - it copies the log file previously saved in plugin workspace area. If that file does not exist (or project is not selected) the action should be in disabled state. Also, it should listen to preference changes in case a user changes log file in project properties. Not sure how easy it is to make proper disablement of others. - You should not introduce another name for the console. Can't you get it programmatically from IConsole? - As the console name appears in the UI console list now - I think "C-Build" is too brief. Compare with "Java Stack Trace Console" for example. I suggest to change it to "CDT Build Console". - You need to add eclipse copyright header to the new file BuildConsoleFactory.java
(In reply to comment #6) > (In reply to comment #5) > > Created an attachment (id=175452) > > Better fix > Thanks, don't see the exceptions now. Here are some more comments: > - The message popping up on "Copy Build Log" is not accurate. It says "A project > must be built to be able to save its build log." The actual problem is that no > project is selected. That is right. However, when the user simply selects a project without building it and press "copy log", they'll get a second popup saying that "No build was logged. [...]" I thought that the first popup should indicate what the user should do to avoid getting a new popup. > I'd rather not to have any popup at all here. One could > hardly expect to copy any log in this case. But one could expect the action to > be disabled, that's true. I think no popup is sufficient then. The console will be blank anyway. > > Should the above actions really be enabled when they won't work? If there is > not error or warning in the console, maybe we should disable the next/previous > error actions. > - I think it should be possible to disable "Copy Build Log". What it does - it > copies the log file previously saved in plugin workspace area. If that file does > not exist (or project is not selected) the action should be in disabled state. > Also, it should listen to preference changes in case a user changes log file in > project properties. Not sure how easy it is to make proper disablement of > others. Could this be done is a separate bug since the preference suggestion is valid even without my change? > - As the console name appears in the UI console list now - I think "C-Build" is > too brief. Compare with "Java Stack Trace Console" for example. I suggest to > change it to "CDT Build Console". Good idea. > - You should not introduce another name for the console. Can't you get it > programmatically from IConsole? Where am I introducing the name? Do you mean plugin.properties? Above you suggest a new name, so I'm confused :-) I needed the name in the plugin.xml file, so I had to hard-code it. > - You need to add eclipse copyright header to the new file > BuildConsoleFactory.java As a committer, I am ashamed to have made this mistake :-o Patch coming.
Created attachment 175610 [details] Fixed after comments This patch should address Andrew's comments.
(In reply to comment #7) > > > Should the above actions really be enabled when they won't work? If there is > > not error or warning in the console, maybe we should disable the next/previous > > error actions. > > - I think it should be possible to disable "Copy Build Log". What it does - it > > copies the log file previously saved in plugin workspace area. If that file does > > not exist (or project is not selected) the action should be in disabled state. > > Also, it should listen to preference changes in case a user changes log file in > > project properties. Not sure how easy it is to make proper disablement of > > others. > Could this be done is a separate bug since the preference suggestion is valid > even without my change? Sure but you can't blame me for trying ;). I'll create a bug for that. > > - As the console name appears in the UI console list now - I think "C-Build" is > > too brief. Compare with "Java Stack Trace Console" for example. I suggest to > > change it to "CDT Build Console". > Good idea. OK, what about an idea to change it also in BuildConsoleFontDefinition.description and BuildConsoleFontDefinition.label? > > - You should not introduce another name for the console. Can't you get it > > programmatically from IConsole? > Where am I introducing the name? Do you mean plugin.properties? > Above you suggest a new name, so I'm confused :-) > I needed the name in the plugin.xml file, so I had to hard-code it. Ouch, I guess I got it backwards. I mean, there is BuildConsole.name=C-Build in CPluginResources.properties. Can it be retired in favor of the new console name defined in plugin.xml?
(In reply to comment #9) > (In reply to comment #7) > > > > Should the above actions really be enabled when they won't work? If there is > > > not error or warning in the console, maybe we should disable the next/previous > > > error actions. > > > - I think it should be possible to disable "Copy Build Log". What it does - it > > > copies the log file previously saved in plugin workspace area. If that file does > > > not exist (or project is not selected) the action should be in disabled state. > > > Also, it should listen to preference changes in case a user changes log file in > > > project properties. Not sure how easy it is to make proper disablement of > > > others. > > Could this be done is a separate bug since the preference suggestion is valid > > even without my change? > Sure but you can't blame me for trying ;). I'll create a bug for that. Thanks > > > - As the console name appears in the UI console list now - I think "C-Build" is > > > too brief. Compare with "Java Stack Trace Console" for example. I suggest to > > > change it to "CDT Build Console". > > Good idea. > OK, what about an idea to change it also in > BuildConsoleFontDefinition.description and BuildConsoleFontDefinition.label? I saw those too, but I thought it would make the text a bit long. But if you think it is ok, I'll make the change. > > > - You should not introduce another name for the console. Can't you get it > > > programmatically from IConsole? > > Where am I introducing the name? Do you mean plugin.properties? > > Above you suggest a new name, so I'm confused :-) > > I needed the name in the plugin.xml file, so I had to hard-code it. > Ouch, I guess I got it backwards. I mean, there is BuildConsole.name=C-Build in > CPluginResources.properties. Can it be retired in favor of the new console name > defined in plugin.xml? If I knew how, I would have done it :-) Is there a way for a java class to use a string defined in plugin.properties? I don't think I've see this done before.
(In reply to comment #10) > (In reply to comment #9) > > (In reply to comment #7) > > > > - As the console name appears in the UI console list now - I think "C-Build" is > > > > too brief. Compare with "Java Stack Trace Console" for example. I suggest to > > > > change it to "CDT Build Console". > > > Good idea. > > OK, what about an idea to change it also in > > BuildConsoleFontDefinition.description and BuildConsoleFontDefinition.label? > I saw those too, but I thought it would make the text a bit long. But if you > think it is ok, I'll make the change. Yeah, the texts get 2 characters longer. I think it's OK. > > > > - You should not introduce another name for the console. Can't you get it > > > > programmatically from IConsole? > > > Where am I introducing the name? Do you mean plugin.properties? > > > Above you suggest a new name, so I'm confused :-) > > > I needed the name in the plugin.xml file, so I had to hard-code it. > > Ouch, I guess I got it backwards. I mean, there is BuildConsole.name=C-Build in > > CPluginResources.properties. Can it be retired in favor of the new console > name > > defined in plugin.xml? > If I knew how, I would have done it :-) > Is there a way for a java class to use a string defined in plugin.properties? I > don't think I've see this done before. Sure there is. Here is a snippet: public class BuildConsoleFactory implements IConsoleFactory { private static String consoleName=null; ... public static String getConsoleName() { if (consoleName!=null) return consoleName; IExtensionPoint extension = Platform.getExtensionRegistry().getExtensionPoint(ConsolePlugin.getUniqueIdentifier(), "consoleFactories"); //$NON-NLS-1$ if (extension != null) { IExtension[] extensions = extension.getExtensions(); for (IExtension ext : extensions) { IConfigurationElement[] configElements = ext.getConfigurationElements(); for (IConfigurationElement configElement : configElements) { String consoleClass = configElement.getAttribute("class"); //$NON-NLS-1$ if (BuildConsoleFactory.class.getCanonicalName().equals(consoleClass)) { consoleName = configElement.getAttribute("label"); //$NON-NLS-1$ return consoleName; } } } } consoleName = CUIPlugin.getResourceString("BuildConsole.name"); //$NON-NLS-1$ return consoleName; } } I think it is marginally better. Even if "BuildConsole.name" is still there, it won't be invoked. If you think it's over-complicated, I am OK with just changing the name in the properties file to "CDT Build Console". Your changes are good, feel free to commit. I have no more comments.
(In reply to comment #11) > > > OK, what about an idea to change it also in > > > BuildConsoleFontDefinition.description and BuildConsoleFontDefinition.label? > > I saw those too, but I thought it would make the text a bit long. But if you > > think it is ok, I'll make the change. > Yeah, the texts get 2 characters longer. I think it's OK. That's funny. I was thinking that replacing "C-Build" with "CDT Build Console" would make things longer. But after your comment I realized that it would actually be replacing "C-Build console", which, as you point out, is only two characters. I'll change it :-) > > > > > - You should not introduce another name for the console. Can't you get it > > > > > programmatically from IConsole? > > > > Where am I introducing the name? Do you mean plugin.properties? > > > > Above you suggest a new name, so I'm confused :-) > > > > I needed the name in the plugin.xml file, so I had to hard-code it. > > > Ouch, I guess I got it backwards. I mean, there is BuildConsole.name=C-Build in > > > CPluginResources.properties. Can it be retired in favor of the new console > > name > > > defined in plugin.xml? > > If I knew how, I would have done it :-) > > Is there a way for a java class to use a string defined in plugin.properties? I > > don't think I've see this done before. > Sure there is. Here is a snippet: > public class BuildConsoleFactory implements IConsoleFactory { > private static String consoleName=null; > ... > public static String getConsoleName() { > if (consoleName!=null) > return consoleName; > > IExtensionPoint extension = > Platform.getExtensionRegistry().getExtensionPoint(ConsolePlugin.getUniqueIdentifier(), > "consoleFactories"); //$NON-NLS-1$ > if (extension != null) { > IExtension[] extensions = extension.getExtensions(); > for (IExtension ext : extensions) { > IConfigurationElement[] configElements = > ext.getConfigurationElements(); > for (IConfigurationElement configElement : configElements) { > String consoleClass = configElement.getAttribute("class"); > //$NON-NLS-1$ > if > (BuildConsoleFactory.class.getCanonicalName().equals(consoleClass)) { > consoleName = configElement.getAttribute("label"); > //$NON-NLS-1$ > return consoleName; > } > } > } > } > consoleName = CUIPlugin.getResourceString("BuildConsole.name"); > //$NON-NLS-1$ > return consoleName; > } > } > I think it is marginally better. Even if "BuildConsole.name" is still there, it > won't be invoked. If you think it's over-complicated, I am OK with just > changing the name in the properties file to "CDT Build Console". Normally, I would want to unify the strings, but I think that adding all this code is more confusing for maintenance that the advantage. I'll just change BuildConsole.name. > Your changes are good, feel free to commit. I have no more comments. Thanks a lot for the quick reviews. I will commit later today.
Created attachment 175775 [details] Final fix This is the final patch after Andrew's reviews. Committed to HEAD.
Fixed
*** cdt cvs genie on behalf of mkhouzam *** Bug 320765: Add the CDT build console to the set of consoles that can be opened manually. This is useful so that we can have access to the console before doing a build. [*] CopyBuildLogAction.java 1.2 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/buildconsole/CopyBuildLogAction.java?root=Tools_Project&r1=1.1&r2=1.2 [*] BuildConsolePage.java 1.28 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/buildconsole/BuildConsolePage.java?root=Tools_Project&r1=1.27&r2=1.28 [*] ShowErrorAction.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/buildconsole/ShowErrorAction.java?root=Tools_Project&r1=1.2&r2=1.3 [*] ConsoleMessages.properties 1.12 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/buildconsole/ConsoleMessages.properties?root=Tools_Project&r1=1.11&r2=1.12 [+] BuildConsoleFactory.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/buildconsole/BuildConsoleFactory.java?root=Tools_Project&revision=1.1&view=markup [*] CPluginResources.properties 1.66 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/CPluginResources.properties?root=Tools_Project&r1=1.65&r2=1.66 [*] plugin.properties 1.203 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/plugin.properties?root=Tools_Project&r1=1.202&r2=1.203 [*] plugin.xml 1.382 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/plugin.xml?root=Tools_Project&r1=1.381&r2=1.382
I've added this change to the New and Noteworthy for 8.0, at http://wiki.eclipse.org/CDT/User/NewIn80#Console
