| Summary: | No feedback when launch is waiting for build | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Jared Burns <jared_burns> |
| Component: | Debug | Assignee: | Darin Wright <darin.eclipse> |
| Status: | VERIFIED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | darin.eclipse, erich_gamma, philippe_mulet |
| Version: | 3.1 | ||
| Target Milestone: | 3.1 M5 | ||
| Hardware: | PC | ||
| OS: | All | ||
| Whiteboard: | |||
|
Description
Jared Burns
The "background" progress dialog should appear in this case. Suggest not to add a dummy config. The dialog used to appear, not sure what changed. I've played around with this some more, and the dialog does appear if you have the preference set to "prompt" to wait before launching. In this case, we first prompt with a yes/no/cancel and then the progress dialog appears which lets you select "Run in Background." It makes sense that no dialog appears with the preference set to "always", but we need to provide some feedback in the Debug view. This is why we put the "dummy" launch in the Debug view. In my opinion, the fix to the prompt issue is just a clarification of our test script. But the missing launch bug should be fixed. Is it possible that this bug was caused by the move to a background content provider? For reference, the "dummy config" was added to fix bug 49972. I think it's very important that we keep (fix) this feature. Yes, we added the dummy launch to fix bug 49972, but when the new background progress reporting arrived (late 3.0), we migrated to use it. This problem was reportedly fixed - see bug 57306. I argue to be consistent, we should always show the dialog if they said "always wait for build". The user is then able to press "Run in Background" on the dialog to make it go to the background. Hmm. It seems strange to me that you would pop a dialog up when someone has explicitly told us not to prompt. I don't feel too strongly about that, though, and it doesn't really matter for the dummy launch node. If the user turns on the preference to "always run in background," they won't get the progress dialog to "run in background". We must provide the dummy launch so the user knows that the launch is going. Otherwise, the user clicks Run/Debug and nothing happens. Can I fix this? My inclination is not to show a dummy launch object. I'd like to get the input from Erich and Philippe. adding as CC for comment. When "always run in background" is selected, I should be allowed to trigger an action, and not be prompted until it completes(unless some deadlock occurred). The option to "wait until build is finished" seems quite orthogonal to me; it does not imply that I be prompted. It can wait in background if all operations to be run in background. So I would vote for not prompting and showing dummy launch config; as I think the "always run in background" setting is a master switch turning off most interactions. One thing to consider is that the debug view may not even be visible from the location the user decides to launch. For example, I often launch from the Java perspective and wait for a breakpoint to be hit (to auto switch to the debug view). Users who do this will never see the dummy launch. Instead, I use the progress view. By default, a perspective switch does not occurr when launching. I see. But is it much different from the case where in this mode, it takes a while before you hit the breakpoint (no matter if it needs to build or not). Isn't the dummy launch config echo'ed in the progress view ? When the user has set the global preference to "always run in background" then it is better to now show the progress dialog. The rationale is that when users have actively set this preference, then they know how to find out about progress, i.e., by checking the progress view. Conclusions: * the style for showing progress is via the blocking progress dialog when "always run in background" workbench preference is "off" * the style for showing progress is via the progress view when the "always run in background" workbench preference is "on" Currently, the debugger only shows the blocking progress dialog when "always run in background" is off, and the user has asked to be prompted to wait for builds. I think the dialog should also be shown when the user has selected "always" wait for builds and "always run in background" is off, because the workbench preference controls when the blocking progress dialog appears, and the lanuch preference simply controls whether to wait for the build (not how to show progress). We don't need to bring back the dummy launch - we intentionally removed that support when migrating to show progress of background jobs. In this test case, the user has told us two things: 1. Launch in the background. 2. Always run background jobs "in the background" Popping up a dialog in this case is completely contrary to the users explicit requests. Correct. In that case no dialog is shown. We show a dialog when the preference is "launch in backgroud" and "always run in background" is OFF. I read one of your sentences backwards, sorry. :) Anyway, the state we're left in at the end is that bug 49972 was reintroduced. Can you please explain how this bug is an improvement? Bug 49972 is *not* re-introduced. We do show feedback when launching - via the progress dialog or via the progress view (which is the way endorsed way feedback is presented in Eclipse 3.0 and greater). Just to drive the point home further... 1. The current situation is that the user clicks the "Debug" or "Run" button while a build is happening and nothing happens. This is a GUI blooper. 2. The Progress view is not visible by default in the workbench. Nor is it intended as a required punishment (in wasted real estate) for users who choose to "Always run in background." Because the view is not visible, it is not correct to say that we provide feedback via this view. 3. Nothing else in Eclipse behaves this way. Our behavior *was* consistent with the rest of the workbench. To make the rest of the workbench consistent with our new behavior we would have to: a. Remove the build feedback from the status bar b. Remove the CVS Resource History feedback from the status bar c. Remove the "pending..." dummy node from deferred tree content providers d. Remove the "Searching..." dummy node from the Search view e. etc. In all of these places, when the user chooses to perform an action, the background job would be reported in the Progress view. I don't understand why we're even debating this. Can you please explain how removing the feedback from the Debug view is a feature worth fighting for? As you pointed out, when you launch and a build is in progress, there is feedback in the progress bar - i.e. that a build is currently in progress, which explains why the launch is pending. For an advanced user, this might not be a problem. You click Debug, nothing happens, but you notice the "Building..." in the bottom right of the workbench and remember how the options have been configured. But for a newer user (or someone who isn't thinking about how their options interact), there's no explicit connection between the "Building..." progress that's ongoing in the status bar and the lack of feedback after they click Run or Debug. The result (as reported even by advanced users in bug 49972) is that the user ends up clicking the Run/Debug button multiple times, "secretly" queueing up a series of duplicate launches. With the "dummy" node in place, this problem goes away because the user gets immediate (and contextually meaningful) feedback in the view when they press Run/Debug. Suggest to add a message to the debug view, similar to the search view's "Searching" message that appears when a search has started but there are no results. The message should appear as the first element in the debug view, in italics - something like "Launching - waiting for build". What we had was your suggestion++. My implementation provides the actual named of the config to the user so they know exactly what is launching, if they queue up multiple launches that is readily visible, they had pointers to the config in the context menu, etc. Please explain the problems with my previous arguments. Alternatively, please explain the problems with the code we had. Please investigate if the real launch can be used, and have it wait for the build before launching, rather than using a dummy launch object. Do use the real launch, the platform's implementation if ILaunchConfiguration.launch(...) would have to change to create the launch and return it before telling the delegate to launch. This would seem to violate the API specified on ILaunchConfiguration.launch(...) which makes it clear that this method tells the delegate to launch and then returns the launch. Potentially reversing the timing on this, could have catastrophic effects for clients. For example, if a client calls ILaunchConfiguration.launch(...) and then tries to access the children of the launch, there won't be any. There's no problem like this using a dummy launch. I looked at moving the wait code into the LaunchConfiguration.launch(...) implementation. However, it won't work because we can't prompt from the background thread and the preference is (importantly) a yes/no/prompt setting. Can't the prompt be implemented with a status handler similar to the "launch with compile errors" prompt? After spending too much time trying to figure out how to get my prompter to actually prompt (I didn't notice the double-status-handler trick that the "launch with compiler errors" handlers do to get into the UI thread), I've hit the deadlock problem that I mentioned in comment #24. By moving it down into the config, we introduce deadlocks. Launching while doing a build introduces a deadlock with external tool builders, for example. A build starts, then the external tool builder tries to launch the config, but the launch waits for the build to finish and blamo. Another (much less severe) problem with using the actual launch is that we can't append any useful hint like "(waiting for build...)" to the end of the name like we can with a dummy launch. Also, I'd like to be able to make the dummy node a subclass of Launch that can handle termination without debug targets/processes so the user has the option of terminating the launch while it's waiting for a build to finish. The deadlock could be avoided by adding a new launch method with an extra boolean parameter to the launch method indicating if the launch should wait for a build. DebugUITools would use the user pref to set the value, and an external tool builder would just specify "false". (By default, the value would be false, when the client calls a launch method that does not have the parameter, to be backwards compatible). However, the special "terminate" behavior and label would require other hooks. Since a client could provdie a launch implementation, this is also problematic (i.e. so they get the same terminate behavior of cancelling the launch). If we had not allowed clients to provide launch implementations, this could have been fixed internally without effecting them. Dummy launch avoids this problem. Fixed in DebugUIPlugin. I just added the few lines of code back to use a dummy launch with "(waiting for build...)" in the label. If we remove the "final" declaration(s) on Launch (bug 84189), I'll then go ahead and implement the support for termination while the launch is in the waiting state. Please verify, DW. |