Created attachment 114324 [details]
Does not work as you might want - pauses for 10s
In an asynch ChildFactory, createNodeForKey is supposed to be fast, and createKeys often just runs through one loop (according to an underlying model) and then returns true. The problem with this style is that the explorer just shows a "Please wait..." node until the list is fully populated, which can take a long time.
It also supports returning false to "create more keys", but this does not work very well. In order to add more keys on the end of the list, you need to know what position in the list you are in when createKeys is called. This means you need to manually convert a natural loop into continuation-passing style.
But it is even worse than that, because you cannot just store the current index (say) in a field in the factory - AsynchChildren.run might halt the computation in response to an interrupt and restart it later, without giving any indication to the factory that this has happened. The only way to be sure you are continuing at the right place is to ensure that each loop iteration produces a predictable number of keys (one, even if createNodeForKey will return null later) and count the size of the existing list; or otherwise inspect the last element in the existing list.
This gets very awkward, and can only work if the setup for the loop is very fast and creating each key on a loop iterator is the only significant job. f19dc2ad222e shows how this can be done (for a Maven POM project's submodule list); the final patch looks short but getting there involved a lot of trial and error. bdf59cfe5ce5 is an example where it is necessary to create keys which will never be displayed, just so that the list size matches the iterator index, in some cases even requiring a new key type just to hold this information (FileKey). 5ebd3e1ba751 is an example that is probably wrong (will not handle interruption and restart correctly), though it is hard to know how to test this.
Created attachment 114325 [details]
Workaround - manually translated to CPS
Another minor issue with the known workaround is that the Please wait... node disappears after the first key is produced, which leaves no indication other than the wait cursor that more keys are coming.
Created attachment 114327 [details]
decreasing version number looks a little bit strange, although I understand that module itself was changed 7.26->7.27, how it worked with 7.9 (7.5 in other changed place) at all?
Y01 Stop class could be STOP instance, I think.
Y02 What about addAll? Should not it be atomic operation?
Y03 Write a bunch of tests. Otherwise I don't want such code in module that I maintain.
(In reply to comment #5)
> decreasing version number looks a little bit strange
Sorry, I have no idea what you meant by this comment. What is decreasing? openide.nodes is going from 7.26 to 7.27, and all modules using ChildFactory and now taking advantage of the new behavior are updating their dependency accordingly.
Y01 - OK.
Y02 - you mean, should addAll immediately display the newly added keys? It could, I just did not bother since known usages would be calling add(E) on one key at a time.
(Typically you would use addAll if you had a bunch of keys ready-made from some part of the model, in which case createKeys would be ending after this call anyway, and you would not care about this RFE. In principle some factory might somehow slowly generate a collection of keys, addAll, then slowly generate another batch, and so on; seems pretty unlikely but could be implemented easily enough if you care.)
Currently with the patch addAll will work as before - the keys will be appended to the list but the display will not change until you return from createKeys.
Y03 - will try.
Also need to handle nulls being temporarily added to the list (even if subsequently removed):
(In reply to comment #7)
> (In reply to comment #5)
> > decreasing version number looks a little bit strange
> Sorry, I have no idea what you meant by this comment. What is decreasing?
> openide.nodes is going from 7.26 to 7.27, and all modules using ChildFactory
> and now taking advantage of the new behavior are updating their dependency
Sorry, it was my fault (I misinterpret numbers) and your change is correct.
(In reply to comment #6)
> Y01 Stop class could be STOP instance, I think.
It cannot, because I need to catch it by class name.
Integrated into 'main-golden'
User: Jesse Glick <firstname.lastname@example.org>
Log: #206556: asynch ChildFactory has poor support for incremental display.