Community
Participate
Working Groups
1) The clone API should not make any assumptions about which clone container to save the clone too. Instead, I caller should pass in the container to store the clone in, null if the clone need not be stored. Although the current snippet container is an MUIElement, I suspect this should be changed to make clone containers a first class concept. 2) After cloning a perspective there could be an arbitrary number of elements that caller will want to modify in the clone, not just the clone's ID. The clone API should just make, optionally store, and return the clone. The caller will modify the clone as appropriate.
Created attachment 202339 [details] API changes This patch includes the API changes discussed.
Dean, while looking at the patch I realized that we likely want to do something similar to 'cloneSnippet'. The original API had a presumption that the snippetContainer could only ever be MApplicaiton which is incorrect. Do you think it makes sense to change: public MUIElement cloneSnippet(MApplication app, String snippetId) to public MUIElement cloneSnippet(MUIElement snippetContainer, String snippetId) If so could you make a revised patch with both ?
Yes I think it makes sense.... However, overall, I think I would prefer the bigger change making SnippetContainer a real concept, and getting it off of MUIElement. It seems to me that there are lots of live MUIElements in memory at any given time, and even saving the few bytes needed to hold an empty snippetsContainer slot might be a moderate savings. This also made me think, and then confirm, that deleting user saved perspectives does not work. And that, among other changes, we will need some API do delete snippets. I created a new defect and assigned it to myself. I'll work on this today. https://bugs.eclipse.org/bugs/show_bug.cgi?id=356307
Committed in >20110901. commit a22bb68dab89f4a1fa3ae84d05e3cff95fca0b16 While there is no explicit 'deleteSnippet' this functionality is trivial to implement using just the EMF calls. We will be reviewing whether *every* ui element needs to be a snippet container later in 4.2...as per Bug 356307.
Verified with I20110705-1340 by exercising the save perspective code.
Cut and paste error in previous comment. This was verified in I20110915-0200