Bugzilla – Full Text Bug Listing
| Summary: | Synchronize Action should display "All models" by default | ||
|---|---|---|---|
| Product: | [Technology] EGit | Reporter: | James Blackburn <jamesblackburn+eclipse> |
| Component: | UI | Assignee: | Project Inbox <egit.ui-inbox> |
| Status: | NEW --- | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | dariusz.luksza, matthias.sohn, mn, stefan.lay |
| Version: | 0.10.0 | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | All | ||
| Whiteboard: | |||
|
Description
James Blackburn
+1 Especially if you want to preview uncommitted changes the current behaviour is unhandy. I'm also more interested in the overall diff between the current workspace state and a tag or branch. The list of commits can be seen in the history view. Should we change the default to "all models" after we also present staged and not staged files in git change set model? This is still a major PIA. Dariusz, I still don't see the point of the "Git Change Set" model. It lets you see the individual commits, but you can't do anything with them. The History View is just as useful for seeing the changes made in a commit, and opening a compare editor on the changeset, isn't it? Coming from other integrated VCSs, the Synchronize perspective is used for staging and committing changes to the current HEAD. The Workspace model is then perfect for seeing changes resources, editing them, staging them, etc. See also bug 327077 In bug 323839 you alluded that the performance issues are due to fetching the patchsets -- as diff'ing two HEADs should be fast, right? If this is the case then I'd think that the changes for the commits shouldn't be listed by default. Instead the Synchronize view should contain: Workspace Model: <Project> <Folder> <File> <Project2> ... Git Change Set: <Working Tree> <Staged Changes> Where working tree and staged changes are as they are today. +1 for changing the default to "All Models" or even to "Workspace" model. The first thing we do in there is to select it (from one of the 2 identical dropboxes on the view toolbar). Additionally, in initial synchronize dialog the destination branch should be <local branch> HEAD by default. I believe it is the most common way of using Synchronize View to check on the differences (and fix little things) before commit. (In reply to comment #4) > +1 for changing the default to "All Models" or even to "Workspace" model. The > first thing we do in there is to select it (from one of the 2 identical > dropboxes on the view toolbar). The two identical drop downs seems to be new in 0.12.XX, it wasn't there in 0.11.3 afaik. > Additionally, in initial synchronize dialog the destination branch should be > <local branch> HEAD by default. I believe it is the most common way of using > Synchronize View to check on the differences (and fix little things) before > commit. Agreed. It currently defaults to something random. Synchronize should be all about the local HEAD, and that should be the default, like CVS. Like CVS it probably shouldn't display the dialog, it should actually just do the synchronize assuming Workspace vs HEAD... It may sometimes be useful to compare with another branch / tag, but there are other actions for that. Compare With > .... I think the compare with action should also reuse the synchronize view (bug 327077), so that there's consistency in UI. Having two ways to perform the same operation is confusing when one of them contains all the functionality of the other. I still don't get how the commits in the Git Change Set model are useful, apart from the special 'staged', and 'workspace modified' elements. Everyone using egit here has been confused by it, and if it makes the simple diff of two heads really slow, it's even worse. git diff without this should be really *fast*: bug 323839 comment 1 (In reply to comment #5) > Synchronize should be all about the local HEAD, and that should be the default, > like CVS. Like CVS it probably shouldn't display the dialog, it should actually > just do the synchronize assuming Workspace vs HEAD... > It may sometimes be useful to compare with another branch / tag, but there are > other actions for that. Compare With > .... I think the compare with action > should also reuse the synchronize view (bug 327077), so that there's consistency > in UI. Having two ways to perform the same operation is confusing when one of > them contains all the functionality of the other. I agree with that too that it would be handy to have menu item to Synchronize with HEAD directly with no extra dialog. And that "Synchronize With..." should be separate and it corresponds to "Compare With" in CVS. Although I feel in CVS plugin location of "Compare With" would be more natural under Team submenu, I always trip up over it. (In reply to comment #6) > And that "Synchronize With..." should > be separate and it corresponds to "Compare With" in CVS. Although I feel in CVS > plugin location of "Compare With" would be more natural under Team submenu, I > always trip up over it. Agreed. Compare with and Replace with seems to be top-level submenu's, so it would likely need to exist in both. Also I think "Compare With..." is better name than "Synchronize With..." as 'Synchronize' suggests that you can take actions like commit or update, whereas I don't think this is possible in egit except for the special case of comparing with 'HEAD'. Behind having "synchronization dialog" is idea of synchronizing with other remote branches eg. from github or private repository of particular user. In such situation you can clearly see what changes was made. In Git Change Set model you can see all incoming commits, you can also check what changes was made in particular file in particular commit. You can also switch into workspace model and review "flatten" changes. The second use case scenario for having commits in Git Change Set model is when you have a set of commits that you want to push into remote, in this case you can also once again review it. Another one is when you fetch changes from upstream and want to review it, maybe pick a particular incoming commit and rebase onto it. I understood that currently this perspective seams to be useless because you can't make any action from it, but this is only a matter of time when such actions like 'merge into', 'rebase', 'carry pick' and so on would be added. But maybe you have a right that this functionality should be renamed to 'Compare with...' to keep backward compatibility with CVS approach. In my opinion the synchronization view is something more then only a place where you can review current changes and prepare a commit this is also a place where you can manage yours repository. Of course preparing commit would be most often used therefore I can agree that we can re-enable workspace model as default perspective. But we should remember that there are lots of differences beaten git and cvs workflow that we shouldn't reduce or hide. (In reply to comment #8) > Behind having "synchronization dialog" is idea of synchronizing with other > remote branches eg. from github or private repository of particular user. In > such situation you can clearly see what changes was made. In Git Change Set > model you can see all incoming commits, you can also check what changes was > made in particular file in particular commit. You can also switch into > workspace model and review "flatten" changes. The second use case scenario for > having commits in Git Change Set model is when you have a set of commits that > you want to push into remote, in this case you can also once again review it. > Another one is when you fetch changes from upstream and want to review it, > maybe pick a particular incoming commit and rebase onto it. > > I understood that currently this perspective seams to be useless because you > can't make any action from it, but this is only a matter of time when such > actions like 'merge into', 'rebase', 'carry pick' and so on would be added. Well these are interesting use-cases, and I agree it could be useful. However there are a few things worth bearing in mind: Usability: keep it simple for the use cases used 99% of the time. I really believe that users will be comparing complete-tree changes between two heads. Why? Because this is how people work in real life: - they merge heads, not arbitrary commits - they rebase against a head, not an arbitrary commit - (and cherry picks, while useful, are generally done between branches which aren't merged...). - If branches have diverged by more than a handful of commits then it's all but useless as the user as they can't get a sense of how the overall tree will change when they merge without looking at the complete diff. The above matches the workflow I see when using git CLI. In general merge, push, pull are operating on HEADs, and users care about the end result, frequently not how they got there. I agree the individual commits are useful and the history view gives us that. I just don't see that the existing default presentation is helpful to the average developer. Does any other tool present synchronize like this? > But we should remember that there are lots of differences > beaten git and cvs workflow that we shouldn't reduce or hide. Agree that there are differences, but disagree that we shouldn't try to reduce complexity. To get people on board, the UI needs to be straightforward for the usecases and workflows users will use day-to-day. The current content of the Synchronize view seems (to me) complicated by these commits that aren't useful for my normal workflow. Moreover it's hard to explain to new users (and experience git CLI users) how to use egit. Trying to explain the flow: checkout, hack, commit, push. All eclipse committers need to do this and they do this all day long. Now how does the current synchronize view fit in? The current UI, in its default state, doesn't help with the above flow, does it? Moreover if building this data is slow, then I'd argue shouldn't be done by default. By all means have a Commits group in the synchronize tree, but if it's expensive to compute then it should be computed lazily, don't you think? A lot of eclipse is about moderating the user experience to provide an experience that is powerful and accessible. Clearly pros should have access to the information, but new users shouldn't be over-whelmed. OK, James you convince me, but you must be aware of one disadvantage of Workspace model. It doesn't include non-workspace files. For some projects this isn't a disadvantage but eg. for EGit and JGit it a valid disadvantage, because we keep our master pom.xml file in repository root with isn't included in eclipse workspace. Therefore any incoming changes in our master pom file wouldn't be show in synchronization results. Workspace model since it is a *workspace* model it will never show non-workspace files, but Git Change Set model show them. I'm currently fixing test cases for synchronize view after switching on workspace model and will push whole patch set in upcoming hours. (In reply to comment #10) > OK, James you convince me, but you must be aware of one disadvantage of > Workspace model. It doesn't include non-workspace files. For some projects this > isn't a disadvantage but eg. for EGit and JGit it a valid disadvantage, because > we keep our master pom.xml file in repository root with isn't included in > eclipse workspace. Therefore any incoming changes in our master pom file > wouldn't be show in synchronization results. > > Workspace model since it is a *workspace* model it will never show > non-workspace files, but Git Change Set model show them. I agree this is a major issue. Users need to be able to find out about resource changes in the repo. but not visible under the workspace. I don't think changeset model addresses this as, if there are 100 changes, the user might very easily miss the file... Perhaps this might require a change to team API, or maybe there's an inventive way to workaround it. > I'm currently fixing test cases for synchronize view after switching on > workspace model and will push whole patch set in upcoming hours. Ok, great. Note, I agree with your points, I'm just keen the UI workflows are user friendly. (In reply to comment #11) > Perhaps this might require a change to team API, or maybe there's an inventive > way to workaround it. Maybe instead of Workspace model, there should be a 'Repository model', that should have full functionality of workspace plus it would show non-workspace files. This is (the 'Repository model') in my opinion the way we should implement 'flat' presentation in sync-view and we should remove 'Workspace model' from model list because of it's limitations. What do you think about this idea James? Sounds ideal. The workspace model presumably provides efficiency and sync state persistence benefits, so should be faster than stat'ing the whole tree -- though maybe egit does a full stat anyway? If you could get the benefits of WS sync as well as handle external resources in one model, that would be great. Change #2999[1] enables by default workspace model. [1] http://egit.eclipse.org/r/#change,2999 (In reply to comment #14) > Change #2999[1] enables by default workspace model. > > [1] http://egit.eclipse.org/r/#change,2999 merged this change as 17909ae55052a0856ff7c2a1627bbef5426dce0e showing non-workspace files in synchronize view is still open [Batch change] Remove passed Target Milestones If anyone on CC list is going to fix/implement this, feel free to assign a new, post-1.3/2.0, target milestone. |