Bugzilla – Full Text Bug Listing
| Summary: | [repoView] cleanup "link with editor" and "link with selection" | ||
|---|---|---|---|
| Product: | [Technology] EGit | Reporter: | Matthias Sohn <matthias.sohn> |
| Component: | UI | Assignee: | Dani Megert <daniel_megert> |
| Status: | VERIFIED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | daniel_megert, robin |
| Version: | 1.0 | ||
| Target Milestone: | 3.0 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
|
Description
Matthias Sohn
*** Bug 355704 has been marked as a duplicate of this bug. *** (In reply to comment #0) > Currently the toolbar button "link with editor" and "link with selection" > use the same icon in the repository view. Do we really need to separate > these 2 options ? This is the only view that I know of, which separates this. I would only keep 'Link with Selection', which can track both. https://git.eclipse.org/r/#/c/13339/ hides the unwanted inherited common navigator items, including the useless 'Customize view...' view menu item. (In reply to comment #2) > (In reply to comment #0) > > Currently the toolbar button "link with editor" and "link with selection" > > use the same icon in the repository view. Do we really need to separate > > these 2 options ? > > This is the only view that I know of, which separates this. I would only > keep 'Link with Selection', which can track both. > > https://git.eclipse.org/r/#/c/13339/ hides the unwanted inherited common > navigator items, including the useless 'Customize view...' view menu item. ok, this hides these actions which is nice, but it doesn't yet teach 'Link with Selection' to also 'Link with Editor' (In reply to comment #3) > ok, this hides these actions which is nice, but it doesn't yet teach 'Link > with Selection' to also 'Link with Editor' 'Link with Selection' also links with the editor. Does it not work for you? (In reply to comment #4) > (In reply to comment #3) > > ok, this hides these actions which is nice, but it doesn't yet teach 'Link > > with Selection' to also 'Link with Editor' > > 'Link with Selection' also links with the editor. Does it not work for you? Just compared behavior with and without this change and found you are right, it works :-) Looks like I misinterpreted the failing test. Should we simply delete this test or would it make sense to adapt it for the action 'Link with Selection' ? Please retarget this change to stable-3.0 (In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > ok, this hides these actions which is nice, but it doesn't yet teach 'Link > > > with Selection' to also 'Link with Editor' > > > > 'Link with Selection' also links with the editor. Does it not work for you? > > Just compared behavior with and without this change and found you are right, > it works :-) Looks like I misinterpreted the failing test. Should we simply > delete this test or would it make sense to adapt it for the action 'Link > with Selection' ? > > Please retarget this change to stable-3.0 Adapted test, fixed trailing whitespace and retargeted to stable-3.0: https://git.eclipse.org/r/13487 (In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > (In reply to comment #3) > > > > ok, this hides these actions which is nice, but it doesn't yet teach 'Link > > > > with Selection' to also 'Link with Editor' > > > > > > 'Link with Selection' also links with the editor. Does it not work for you? > > > > Just compared behavior with and without this change and found you are right, > > it works :-) Looks like I misinterpreted the failing test. Should we simply > > delete this test or would it make sense to adapt it for the action 'Link > > with Selection' ? > > > > Please retarget this change to stable-3.0 > > Adapted test, fixed trailing whitespace and retargeted to stable-3.0: > https://git.eclipse.org/r/13487 Thanks Robin! Waiting for a green build, but Hudson seems to be slow/dead. > Thanks Robin! Waiting for a green build, but Hudson seems to be slow/dead.
GitRepositoriesViewTest.testLinkWithSelectionEditor still fails. But IMO it is doing something strange.
First it tests if changing the active editor changes the selection in the repositories view. This part is ok.
Then it tests if changing the selection (only single-click, no double-click) in the repositories view changes the active editor. This part is strange, as e.g. the linking in the navigator view does not work that way.
The second part used to work with the old "Link with Editor" (just reproduced it). The question is, should we actually try to keep that feature or is it OK to remove it?
(In reply to comment #8) > > Thanks Robin! Waiting for a green build, but Hudson seems to be slow/dead. > > GitRepositoriesViewTest.testLinkWithSelectionEditor still fails. But IMO it > is doing something strange. also fails here on Mac > First it tests if changing the active editor changes the selection in the > repositories view. This part is ok. > > Then it tests if changing the selection (only single-click, no double-click) > in the repositories view changes the active editor. This part is strange, as > e.g. the linking in the navigator view does not work that way. The package explorer works this way if the selected file already has an open editor for the files involved. If the file isn't yet open it needs a double click but if there is already an open editor which isn't on the top of the open editors clicking the file in the package explorer will bring the corresponding editor to the top. > The second part used to work with the old "Link with Editor" (just > reproduced it). The question is, should we actually try to keep that feature > or is it OK to remove it? Since package explorer seems to behave this way it would be nice to keep this feature (In reply to comment #9) > (In reply to comment #8) > > > Thanks Robin! Waiting for a green build, but Hudson seems to be slow/dead. > > > > GitRepositoriesViewTest.testLinkWithSelectionEditor still fails. But IMO it > > is doing something strange. > > also fails here on Mac > > > First it tests if changing the active editor changes the selection in the > > repositories view. This part is ok. > > > > Then it tests if changing the selection (only single-click, no double-click) > > in the repositories view changes the active editor. This part is strange, as > > e.g. the linking in the navigator view does not work that way. > > The package explorer works this way if the selected file already has an open > editor for the files involved. If the file isn't yet open it needs a double > click but if there is already an open editor which isn't on the top of the > open editors clicking the file in the package explorer will bring the > corresponding editor to the top. You're right, I must have made a mistake when trying it with navigator, it works now. > > The second part used to work with the old "Link with Editor" (just > > reproduced it). The question is, should we actually try to keep that feature > > or is it OK to remove it? > > Since package explorer seems to behave this way it would be nice to keep > this feature Any ideas, Dani? By the way, the other test failure (HistoryViewTest.testRebaseAlreadyUpToDate) also occurs with other changes and seems to be caused by this SWTBot change, see my comment: https://bugs.eclipse.org/bugs/show_bug.cgi?id=404346#c28 (In reply to comment #9) > (In reply to comment #8) > The package explorer works this way if the selected file already has an open > editor for the files involved. If the file isn't yet open it needs a double > click but if there is already an open editor which isn't on the top of the > open editors clicking the file in the package explorer will bring the > corresponding editor to the top. > > > The second part used to work with the old "Link with Editor" (just > > reproduced it). The question is, should we actually try to keep that feature > > or is it OK to remove it? > > Since package explorer seems to behave this way it would be nice to keep > this feature I agree that enabling 'Link with Selection' in the 'Git Repositories' view should set the selection based on the selection in the previously active part, i.e. also when a file was selected in the 'Package Explorer'. See e.g. how the 'Declaration' view works. I would consider this a separate bug, which can be fixed later as it involves a bit more code: one has to correctly track the previous active selection (provider) to fix this. Nothing one should do when already in RC3 ;-). I would either disable the test or move the whole thing to the next release. (In reply to comment #12) > (In reply to comment #9) > > (In reply to comment #8) > > > The package explorer works this way if the selected file already has an open > > editor for the files involved. If the file isn't yet open it needs a double > > click but if there is already an open editor which isn't on the top of the > > open editors clicking the file in the package explorer will bring the > > corresponding editor to the top. > > > > > The second part used to work with the old "Link with Editor" (just > > > reproduced it). The question is, should we actually try to keep that feature > > > or is it OK to remove it? > > > > Since package explorer seems to behave this way it would be nice to keep > > this feature > > I agree that enabling 'Link with Selection' in the 'Git Repositories' view > should set the selection based on the selection in the previously active > part, i.e. also when a file was selected in the 'Package Explorer'. See e.g. > how the 'Declaration' view works. I would consider this a separate bug, > which can be fixed later as it involves a bit more code: one has to > correctly track the previous active selection (provider) to fix this. > Nothing one should do when already in RC3 ;-). I would either disable the > test or move the whole thing to the next release. Then I'd say lets disable this test for 3.0, accept this change and fix this small issue on master aiming for 3.1 in Sept. (In reply to comment #13) > > I agree that enabling 'Link with Selection' in the 'Git Repositories' view > > should set the selection based on the selection in the previously active > > part, i.e. also when a file was selected in the 'Package Explorer'. See e.g. > > how the 'Declaration' view works. I would consider this a separate bug, > > which can be fixed later as it involves a bit more code: one has to > > correctly track the previous active selection (provider) to fix this. > > Nothing one should do when already in RC3 ;-). I would either disable the > > test or move the whole thing to the next release. > > Then I'd say lets disable this test for 3.0, accept this change Robin, do you have time to upload a new change? > and fix this small issue on master aiming for 3.1 in Sept. Filed bug 409722. (In reply to comment #14) > (In reply to comment #13) > > Then I'd say lets disable this test for 3.0, accept this change > > Robin, do you have time to upload a new change? Change updated. Submitted with http://git.eclipse.org/c/egit/egit.git/commit/?h=stable-3.0&id=d6cf52411377a039fc2906378711091a26e932cb Verified in EGit installed via http://download.eclipse.org/releases/staging/ |