Bugzilla – Full Text Bug Listing
| Summary: | [client] should we continue to permit use of Deferred in hrefCallback of a command? | ||
|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Szymon Brandys <Szymon.Brandys> |
| Component: | Client | Assignee: | Project Inbox <orion.client-inbox> |
| Status: | RESOLVED WONTFIX | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | malgorzata.tomczyk, mamacdon, simon_kaegi |
| Version: | 0.2 | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| Whiteboard: | |||
| Bug Depends on: | 370903, 377035, 377182 | ||
| Bug Blocks: | |||
This already works if your href callback returns the promise. The link will be set when the deferred is done.
We have a hack to handle this, this pattern is repeated several times in the various command methods that generate DOM elements:
href = this.hrefCallback.call(handler, items, this.id);
if (href) {
if (href.then){
href.then(function(link){
anchor.href = link;
});
}else{
anchor.href = href;
}
}
I'd like to leave this bug open because it's not documented and I'm not sure we handle this consistently.
doc bugs to RC1 A lot of our deferred support has been hacked as need in the command framework for demos and such....and often there turns out to be a better way to handle something. So as part of documenting this I want to step back a bit. The deferred href means we could be doing remote calls before the user ever decides to take action, so it's not a good thing. I wonder if we should look at the extension point for "Open With" for guidance post-0.2. Here, a plug-in can map a selection to an href. So the case where we just want to compose an href from selection or contextual information can be handled differently. I'm not sure it would solve Szymon's original case, but I think we need to review the various use cases and see if there are better ways. For RC2 I plan to document the features that we know will stay (like deferred callbacks) and maybe put warnings in ones we aren't so sure of (href) adding doc via bug 343314. Retitling this bug to be a question (to be answered later) marking 0.4. I think Simon is hoping to go in the opposite direction. We've talked about using URI templates for the href callback so that the plug-in never even has to get called at all to generate a link. This is getting more interesting now that we have a "related links" infrastructure. Mapping from a properly formed item to an href should be done via template. There is no reason to load a plugin to do that. BUT what if the item doesn't have the proper decorations from a loaded plugin? In that case, it's not that you want to load the plugin to compute the href, but you might want to do a fetch to get the metadata you need. So I think we want to do a combination of both. The use case is: how do we get from the editor to git status? To do so we need to get item.Parents[0] and then get the metadata needed to understand the git info for that item's parent folder. (In reply to comment #6) > The use case is: > how do we get from the editor to git status? > To do so we need to get item.Parents[0] and then get the metadata needed to > understand the git info for that item's parent folder. I found another way to achieve this. So I think the issue here is, can we decide for sure that we don't need remote hrefs? Seems so but I'm not going to change the code for 0.4. If we support some transforms such as in bug 370903, then I think we could probably stop calling code altogether for hrefs. The work in bug 370903 was to make it easy to calculate hrefs from metadata, so that plugins supply all links declaratively. This removed the main reason for supporting a deferred in an href callback. I had a fix (and even pushed it briefly) but just realized that the original command described by Szymon is still using a deferred to compute an href, so we need to fix the git side. The reason to remove this support is that we don't want to support making expensive/asynch calls to get data just to put a link on the screen, when the user might never care to follow the link. We want to discourage patterns that require retrieving info for every item in a list, just to make a link. Can we revisit the command? What is the missing information that requires an XHR request to build a link between two selected diffs? - can it be represented otherwise? - can the missing metadata be added to the original metadata? (or perhaps it's already there?) On the client side we could say that the command is not a link and then compute a link and open a window when the user clicks it. But that is somewhat bogus. (In reply to comment #8) > The reason to remove this support is that we don't want to support making > expensive/asynch calls to get data just to put a link on the screen, when the > user might never care to follow the link. We want to discourage patterns that > require retrieving info for every item in a list, just to make a link. To clarify..I realize we don't make this xhr call until two commits are selected, but leaving this support in the framework will allow commands that aren't this selective to sneak in. So I think we need to try to find another way. actually, I don't think this command is working at all anyway. opened bug 377035 moving milestone to 2.0 M1 for future triage. Closing as part of a mass clean up of inactive bugs. Please reopen if this problem still occurs or is relevant to you. For more details see: https://dev.eclipse.org/mhonarc/lists/orion-dev/msg03444.html Closing as part of a mass clean up of inactive bugs. Please reopen if this problem still occurs or is relevant to you. For more details see: https://dev.eclipse.org/mhonarc/lists/orion-dev/msg03444.html |
This is a command I would like to add to gitCommand.js. It will be added in the git-log toolbar. var compareGitCommits = new eclipse.Command({ name : "Compare With Each Other", image : "images/git/compare-sbs.gif", id : "eclipse.compareGitCommits", hrefCallback : function(item) { var clientDeferred = new dojo.Deferred(); serviceRegistry.getService("IGitService").then( function(service) { service.getDiff(item[0].DiffLocation, item[1].Name, function(jsonData, secondArg) { clientDeferred.callback"/compare.html#" + (secondArg.xhr.getResponseHeader("Location"))); // secondArg.xhr.getResponseHeader("Location")) <---- this contains URI that I would append later to "/compare.html#" }); }); return clientDeferred; }, visibleWhen : function(item) { if (dojo.isArray(item) && item.length == 2) { return true; }; return false; } }); I think we need something like hrefCallbackDeffered method in eclipse.Command. When value is available, to icon in the toolbar would be added.