Community
Participate
Working Groups
Currently, org.eclipse.debug.ui.EnvironmentTab and CommonTab override activated() and deactivated() not to call their super methods: public void activated(ILaunchConfigurationWorkingCopy workingCopy) { // do nothing when activated } public void deactivated(ILaunchConfigurationWorkingCopy workingCopy) { // do nothing when deactivated } FYI, the super methods call initializeFrom() and performApply(): @Override public void activated(ILaunchConfigurationWorkingCopy workingCopy) { initializeFrom(workingCopy); } @Override public void deactivated(ILaunchConfigurationWorkingCopy workingCopy) { performApply(workingCopy); } I think the tabs should not override the methods so that they call initializeFrom() and performApply(). For example, suppose that another 3rd-party tab makes changes to the environment variable map (the map retrieved by "configuration.getAttribute(ILaunchManager.ATTR_ENVIRONMENT_VARIABLES, ...)"). If a user exists the 3rd-party tab and enters into the EnvironmentTab, the EnvironmentTab does not re-initialize its UI to reflect the enviroment map changes made outside of it. The actual context is here: https://github.com/GoogleCloudPlatform/google-cloud-eclipse/pull/2568#discussion_r150320974
*** Bug 527141 has been marked as a duplicate of this bug. ***
The code of both CommonTab and AbstractLaunchConfigurationTab was added via bug 43952 at same time by same commiter, so I guess the initial intent was not to do anything on activated/deactivated. Wondering if this was really intended to do so or if we have some new use´cases since then?
Perhaps the earlier committer assumed that there will never be inter-tab communication for EnvironmentTab and CommonTab (i.e., no one will ever update environment variables outside the EnvironmentTab). However, but I think that assumption is not realistic. Our plugin is an example, which adds a new tab that modifies environment variables. Again, the context is here: https://github.com/GoogleCloudPlatform/google-cloud-eclipse/pull/2568#discussion_r150320974 For the EnvironmentTab, CommonTab, etc, to correctly display updates made outside them, they need to call initializeFrom() when activated(). And performApply() when deactivated() for a similar reason.
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
Reopening as the bot auto-closed it. Bug still relevant.
Can't you extend the Common Tab and do the necessary stuff in activated/deactivated? It will unnecessarily slow down the switching between the tabs if we add it as a default behaviour.
We are already abusing EnvironmentTab and this looks very hacky: https://github.com/GoogleCloudPlatform/google-cloud-eclipse/blob/f8816f091d41f5b0827919d0d31704f0019652ff/plugins/com.google.cloud.tools.eclipse.appengine.localserver/src/com/google/cloud/tools/eclipse/appengine/localserver/ui/GcpLocalRunTab.java#L333-L342
(In reply to Chanseok Oh from comment #7) > We are already abusing EnvironmentTab and this looks very hacky: > > https://github.com/GoogleCloudPlatform/google-cloud-eclipse/blob/ > f8816f091d41f5b0827919d0d31704f0019652ff/plugins/com.google.cloud.tools. > eclipse.appengine.localserver/src/com/google/cloud/tools/eclipse/appengine/ > localserver/ui/GcpLocalRunTab.java#L333-L342 You can't extend EnvironmentTab to override rather than doing the hack where you are using environment tab?
Of course, I can. And that's another hack. Generally speaking, it's not a good design principle, because it will eventually break in the long run (so will our current hack). Sure, we can copy-and-paste the code blocks of the super methods (i.e., AbstractLaunchConfigurationTab.activated() and deactivate()) and put them into a new tab extending EnvironmentTab. However, the assumption needed is that the code block of AbstractLaunchConfigurationTab.activated() will never change, which can never be true. The code will change (any code will change, that's the well-known principle), and things will break in the future. Sure, you can always hack and make things work, but relying on such an unfounded assumption is a bad practice. That's not the way we build software in our organization. That's why I've filed this issue. I need the right and permanent solution.
Likewise, another assumption needed is that EnviromentTab.activated() and deactivated() will never change to be an empty method. No one can guarantee that these methods will never add any code in the future when new stuff is added to EnvironmentTab.
To reiterate, it will slow up the things as initialzing takes time. so we don't want to di it all the time, will see if something can be defined to decide if it should be called or not.
Thanks, I understand the concern. I just would like to add one more thing for the last moment: IMO, as opposed to AbstractLaunchConfigurationTab, I think EnvironmentTab (as well as possibly other tabs like CommonTab?) isn't doing the right thing. Whenever AbstractLaunchConfigurationTab is (re-)activated, it correctly reconfigures itself and the UI if there were changes to the LaunchConfiguration(WorkingCopy) passed as the argument; AbstractLaunchConfigurationTab does the right thing. OTOH, it looks to me that Environment isn't doing the right thing; it simply ignores changes made to environment variables in LaunchConfiguration when reactivated.
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
