| Summary: | Provide opt-in mechanism for JSDT debug GUI | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [WebTools] JSDT | Reporter: | Jacek Pospychala <jacek.pospychala> | ||||||||
| Component: | Debug | Assignee: | Chris Jaun <cmjaun> | ||||||||
| Status: | RESOLVED FIXED | QA Contact: | Simon Kaegi <simon_kaegi> | ||||||||
| Severity: | normal | ||||||||||
| Priority: | P3 | CC: | cmjaun, Michael_Rennie, pwebster, thatnitind | ||||||||
| Version: | 3.2 | Flags: | thatnitind:
review+
Michael_Rennie: review+ |
||||||||
| Target Milestone: | 3.2.4 | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Linux | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Jacek Pospychala
How does ATF's debugging support handle itself in this situation? I imagine we'd just follow your lead on that. And doesn't most of the ATF UI still refer to Mozilla, i.e. in the perspective name, debug launch shortcut, and launch configurations? The Variables and Debug views would adhere to the correct process, wouldn't it? Nobody ever requested ATF to hide it's debugging features, so there's no way to have ATF without it's debugging. We've been recently reducing "Mozilla" icons and name use because it's implementation detail (quite like "Rhino" in JSDT) and it doesn't need to be so prominent as it was. I like the capabilities idea, so I'd try to cook up a patch in that direction, if you don't object :-) (In reply to comment #2) > I like the capabilities idea, so I'd try to cook up a patch in that direction, > if you don't object :-) I agree with the use of capabilities. We should follow the Platforms' lead on this and provide a capability for the JSDT debug support to completely remove its presence if the user decides to do so. Created attachment 173101 [details]
fix
The patch provides a capability category called "JavaScript Development" with the capability "Debug Support" that can be used to completely disable the JSDT debugger.
Looks good. oh.. is this my birthday today? thanks! (In reply to comment #6) > oh.. is this my birthday today? thanks! Oh right. My +1 is conditional on Jacek showing us his unit test work ;) applied to HEAD and 3.2.1 I don't think this is the correct solution to this problem... I'm not an expert on capabilities, but I thought they were intended to be used at a product level, rather than a plugin level. "First and foremost was our intention that those who are writing plug-ins NOT define activities for them in the plug-in that define their functionality. Any activity definitions provided by the plug-in developer should be isolated so that product developers (those who aggregate plug-ins from various sources) are able to totally own the capabilities for their product." - from article linked at top of this page http://wiki.eclipse.org/Galileo_Capabilities I bring this up because I have run into the same problem as Jacek and want to hide the UI for JSDT debug in an adopting product. Generally this has been done with a product level capability, but I am not able to do that now due to a conflict with the JavaScript Development capability. If anything, defining a JavaScript development capability down in the jsdt.debug plugins doesn't make sense to me. I am opening this bug to suggest removing the capability from the JSDT plugins. Thoughts? Comments? (In reply to comment #9) > I don't think this is the correct solution to this problem... > > I'm not an expert on capabilities, but I thought they were intended to be used > at a product level, rather than a plugin level. > > "First and foremost was our intention that those who are writing plug-ins NOT > define activities for them in the plug-in that define their functionality. Any > activity definitions provided by the plug-in developer should be isolated so > that product developers (those who aggregate plug-ins from various sources) are > able to totally own the capabilities for their product." - from article linked > at top of this page http://wiki.eclipse.org/Galileo_Capabilities > > I bring this up because I have run into the same problem as Jacek and want to > hide the UI for JSDT debug in an adopting product. Generally this has been done > with a product level capability, but I am not able to do that now due to a > conflict with the JavaScript Development capability. > > If anything, defining a JavaScript development capability down in the > jsdt.debug plugins doesn't make sense to me. > > I am opening this bug to suggest removing the capability from the JSDT plugins. > Thoughts? Comments? I have no problem with pushing the capability definition up to JSDT proper. It seems that a project can provide their own capabilities (although suggested it be in an uncategorized feature that doesn't have to be consumed). Is that what you guys read in bug 299344 ? PW Created attachment 189219 [details]
proposed fix
Here is a proposed fix that leaves the activity binding for JavaScript debugging and removes the category and category bindings, that way it is entirely up to consuming products to provide a category / binding.
Adding Chris for review; I'll go with his recommendation. I am still of the opinion that the activity bindings should be removed completely from the JSDT plugins, as I still don't have full control from my product plugin of what is enabled. (In reply to comment #14) > I am still of the opinion that the activity bindings should be removed > completely from the JSDT plugins, as I still don't have full control from my > product plugin of what is enabled. That is fine with me, whatever you think will help the most. Created attachment 191212 [details]
Patch to remove activity
I have submitted a patch that completely removes the activity binding. can you also make a patch for HEAD? hmm, the patch for 3.2.4 does not apply to 3.2.4, there are unresolved segments. Can you re-create the patch Chris? Fix checked into 3.2.4. and HEAD. The patch missed removing the unused NLS strings in the bundle.properties file for the activity and category name / description. I committed these changes as well. (In reply to comment #21) Thanks, Michael. |