Community
Participate
Working Groups
Build ID: e3.3m5 rse2.0m5 Steps To Reproduce: the enablement of "move up/down" in the context menu of connections doesn't cange correctly when connections are moved up or down. e.g. the local connection as first connection after startup always has an enabled move down, even after moving the connection to the second position. the move up action is never enabled.
Kushal -- please fix for RC1 or RC2. We are marking another bug as a duplicate of this.
*** Bug 187866 has been marked as a duplicate of this bug. ***
Dave, I'm assigning this to you. I think the problem is that when we do Move Down, in class SystemMoveDownConnectionAction, it calls moveHosts() on the system registry. This ends up calling moveConnection() in SystemHostPool where I see the following comments. I am not sure why the existing code was commented out. If we uncomment, would the connections be persisted in the correct order so that the ordering is not lost when we come back? Note that right now the connection order does not seem to be persisted. /* * DWD revisit, make sure that connections can be "moved", whatever that means. * It appears that connections can be moved up and down in the list which * probably provides for some rational for keeping this around. */ // java.util.List connList = getHostList(); //FIXME connList.move(newPos, conn); invalidateCache(); }
I don't think that move up and move down should order the hosts in the persistent form of the profile for the following reasons: 1) if we ever support multiple open RSE views I'd want to order things differently in each view 2) if we export and import a profile then I don't want someone else's order imposed on me. 3) having hosts ordered in the profile doesn't really explain what happens if you have multiple profiles on start or restart since there is no "merge" logic. Therefore I think that move up and move down should remain, but affect the view only. If we add or remove hosts by adding or removing profiles those should result in some well-defined presentation on startup consistent with the previous ordering. Possibly there should be a sorting action as well, though that's an improvement for later.
Note that in our commercial product, we're already setting a ViewerSorter into the SystemView because we prefer keeping connections sorted alphabetically and not giving the user the freedom of move up / move down. I've been told that there are some issues with the ViewerSorter not always working reliably, but I thought I'd let you know that there is Eclipse API which allows keeping the Sorter separate from the underlying data model.
Interesting and thanks!
Moving to 2.0.1. Does not require an api change or new PII.
CQ:WIND00097965
We'll need a fix for this for our product. Dave, can you have a look please?
This turns out to be quite a bit more interesting than I anticipated. The current philosophy behind move up and move down is that is moves the connection within the profile. This used to be implemented by actually modifying the profile's ordering of the connections in its persistent form. I believe this is incorrect and inconsistent with the rest of eclipse. I would like to see move up and move down actions in the system view work on all presentations of connections in the workspace and be persistent across sessions, but I can't see what it buys to have this affect the profiles. It seems to me to be incorrect to have my ordering of connections shared with others. I also find the current effect confusing in that it can you can see that a connection is above others and yet can't "move it down" because all the others are in different profiles. There should be one connection ordering for the workspace. New connections should appear at the end of the list. Renamed connections should maintain their place in the list. Deleted connections should be removed from the list. Connections in profiles that are made inactive and then made active again should retain their former position in the list. The same goes for disabling and reenabling system types. The list should be used for all list-like views of connections (drop-downs, system view, system table view, ...). Move up and move down should move the connections in this list regardless of their owning profile. The list should be persisted in the metadata area. I'm not sure it makes sense for it to be a preference since preferences can be exported and imported into other workspaces. It would seem to be a problem to import preferences and find that your connection ordering has changed. I don't believe this is a view property either - hence using a memento to store this associated with the view would not be correct. However, it may be possible to use the underlying property and memento mechanisms to store this in a non-exportable bit of the metadata area. While the ordering of connections may appear to be a UI property, its not clear to me it needs to be. It may make sense to have this be a core property and the API to manipulate this list be a core API. Types affected: SystemView (ui) SystemRegistry (ui) ISystemRegistry (perhaps, core) SystemMoveUpConnectionAction, SystemMoveDownConnectionAction (ui) ISystemViewInputProvider (maybe) I'm marking this as "investigate" for the F2F meeting. Comments?
While I think you're right philosophically, the ordering of connections has been with the profile persisted form in RSE up to now; the move up and down actions do exist; and, in reality, we typically only have one profile. So while I agree with your comments for the longer term, I'd still like to see this issue fixed because it affects our product. People don't like having the move up / move down buttons there but not work properly. Do you have an idea how much effort a fix within the current philosophy might be?
That fix doesn't look to bad so I'll create a patch for it, but I'm not happy with the fact that it only orders inside a profile. We'll have to revisit this. Martin -- please review the patch when attached.
Created attachment 78477 [details] fix for move up/down in host pool
Dave - does this code really work? Did you test it? I see two potential issues: 1.) In moveHosts(): Have connections ABCD, and asked to move BC + 1. You first move B --> ACBD You now move C --> ABCD Result is same as in the beginning, but expected result would be ADBC I'd guess. 2.) In moveConnection(): Have connections ABCD, asked to move B + 1. oldPos = 2 newPos = 3 Remove(oldPos) --> ACD Add(B, newPos) --> ACDB Expected result would be ACBD This can even ArrayIndexOutOfBoundsException I'd guess. Please review this again. It might also help writing a simple unit test for it.
I've committed a fix for this that moves the hosts within the host pool. Unit tests were committed along with it. Martin - please include in the 2.0.1 build if you wish.
The new code looks good as committed. It would have been good to also attach the new fix as a patch.
This problem is fixed in the 2.0.1 driver.
closing
