Bug 111164 - [Change Sets] Select and Commit Multiple Commit Sets at once
Summary: [Change Sets] Select and Commit Multiple Commit Sets at once
Status: ASSIGNED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: CVS (show other bugs)
Version: 3.1   Edit
Hardware: PC Windows 2000
: P4 enhancement (vote)
Target Milestone: ---   Edit
Assignee: platform-cvs-inbox CLA
QA Contact:
URL:
Whiteboard: hasPatch
Keywords: helpwanted
: 74683 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-09-29 19:42 EDT by Will Hains CLA
Modified: 2019-09-06 16:04 EDT (History)
7 users (show)

See Also:


Attachments
handling sets (7.75 KB, patch)
2007-02-16 05:05 EST, Krzysztof Daniel CLA
no flags Details | Diff
Solution proposal (11.00 KB, patch)
2007-02-23 08:31 EST, Krzysztof Daniel CLA
no flags Details | Diff
Non blocking (11.62 KB, patch)
2007-02-28 02:54 EST, Krzysztof Daniel CLA
no flags Details | Diff
Really non blocking (14.68 KB, patch)
2007-03-07 05:59 EST, Krzysztof Daniel CLA
no flags Details | Diff
Deluxe wizard (21.94 KB, image/png)
2007-11-20 10:12 EST, Tomasz Zarna CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Will Hains CLA 2005-09-29 19:42:26 EDT
The commit set feature of the CVS synchronize view is enormously useful for 
keeping commits small and focused when committing a large number of changes. 
But having taken the trouble to assign all the changes to commit sets, the only 
way I can see to commit them like that is to commit each one individually.

Since when you create a commit set you are already creating the CVS comment, I 
don't see why we need to have the commit/comment dialog again, and why each one 
needs to be committed individually.

The way it should work:
1. organise your changes into commit sets
2. select all the commit sets you would like to commit, and choose the commit 
command
--> each commit set is committed separately, with its own existing comment.

To avoid "accidental" commits, we simply need an "are you sure?" prompt.
Comment 1 Michael Valenta CLA 2005-09-30 08:35:00 EDT
This seems reasonable to me. Unfortunately, we don't have the manpower to 
address this in 3.2 but patches will be accepted.
Comment 2 Krzysztof Daniel CLA 2007-02-13 08:55:52 EST
I am trying to write a fix, but I have hit an obstacle.

Committing change sets seems to be easy, if only sets are selected.

However please consider situation, that there is a few of them selected, and a few other resources from other sets (but sets are not selected).

How wizard should work then?
For me it seems to be good idea to expand commit wizard on change sets view to make the user aware, that some resources will be committed with his comment, but change sets with comment assigned to them?
Comment 3 Michael Valenta CLA 2007-02-13 09:21:30 EST
I think the idea behind the request was to skip the wizard entirely when Change sets were selected in the Synchronize view since you already have the comments. In other words, if more than one change set is selected, prompt the user to indicate that the comment from the change sets will be used. If the user clicks OK, simply perform a commit for each change set.

If someone wanted to do a deluxe solution, I could imagine a wizard where the top pane shows the list of change sets being committed and the bottom pane shows the resources being committed. Selecting a change set would only show the resources for that change set in the bottom pane. You could have a third pane containing the comment for the change set. When the user clicks commit, a commit is performed for each change set (so they can each have their own comment).
Comment 4 Will Hains CLA 2007-02-14 03:51:18 EST
Michael's right - I didn't even know there was a Commit Wizard.

How about this for an idea:

Adding a resource to a change set is in effect pre-commenting the change, so that it is not necessary to enter a comment when committing.

Therefore if the user selects a mixture of change sets, resources already added to a change set, and resources not added to any change set, the behaviour should be to commit all resources that are added to a change set with the set's comment, and prompt for a comment for the resources not added to a change set.

If that is too difficult or too confusing for the user (maybe), perhaps the new behaviour is invoked only if the user has selected one or more change sets, but if there is a mixture in the selection it just defaults back to the current behaviour. I think the people that use commit wizards habitually (eg. me) will get what they want, and other people won't even know that anything has changed, so everyone's happy.
Comment 5 Krzysztof Daniel CLA 2007-02-16 05:05:29 EST
Created attachment 59120 [details]
handling sets

If only sets are selected, commit is performed without displaying any dialog. Comments are retrieved from commit sets.
Comment 6 Michael Valenta CLA 2007-02-16 08:59:53 EST
I think we should still have a prompt that explains that the user is committing changes that are part of outgoing change sets. They should have the option to continue, cancel or open the commit wizard anyway. Also, if there are any changes that are not part of a change set, we will need to get a comment for those files by prompting.
Comment 7 Krzysztof Daniel CLA 2007-02-23 08:31:37 EST
Created attachment 59648 [details]
Solution proposal

Current scenario looks like that:
1. user selects some resources (optionally change sets)
2. if any change set is selected, user has to confirm he wants to commit it automatically with previously entered comment. 
2a. if confirmed, change sets are selected
3. commit wizard launches (with all unhandled till now resources)
Comment 8 Michael Valenta CLA 2007-02-23 14:37:41 EST
The general idea looks good. Here are some specific things that need to be fixed:

1) When I apply the patch, I get a compile error (CommitWizard#isAdded is no tvisible). My guess is that you forgot to include a class.

2) You created a new message bundle and class. We general use one per plug-in and for CVS/ui plug-in we use org.eclipse.team.internal.ccvs.ui.CVSUIMessages.

3) You used the SWT MessageBox but that has some shortcomings so you should use the JFace MessageDialog

4) You created an anonymous class for IStructuredSelection but it is better to use the existing StructuredSelection class.

5) When committing the change sets, it looks like you are blocking the user. It would be better to collect the change sets to commit, prompt for any resources that are not part of a change set and then run the entire commit in the background.

Comment 9 Krzysztof Daniel CLA 2007-02-28 02:54:46 EST
Created attachment 59953 [details]
Non blocking

First prompt, then wizard, then committing change sets
Comment 10 Michael Valenta CLA 2007-02-28 10:32:31 EST
There is still a blocking issue in the latest patch. Here's what I see in the patch:

1) The set of change set commit operations is collected and the remaining resources passed to step 2. This step is fine.

2) The CommitWizard is opened for the remaining resources. This will prompt for a comment and run a commit operation in the background.

3) The change set commits are run in a blocking manner (using busyCursorWhile).

I would prefer something like this:

1) Same as above.

2) User is prompted for a comment for the remaining resources. The CommitWizard can still be used but instead of running the operation, the operation is returned to the action.

3) A wrapper action is created that will run all child actions in the background. As an example, look at the AddAndCommitOperation that wraps an AddOperation following by a CommitOperation.

4) The run() (with no arguments) is called on the wrapper action so the operation is performed in the background.
Comment 11 Krzysztof Daniel CLA 2007-03-07 05:59:41 EST
Created attachment 60364 [details]
Really non blocking
Comment 12 Michael Valenta CLA 2007-03-23 15:58:23 EDT
Given that we are nearing the end of 3.3, I am not comfortable with releasing this patch. Adding code that will automatically commit code is just too risky, IMHO. We'll revisit this enhancement early in the next release.
Comment 13 Markus Keller CLA 2007-07-19 07:20:16 EDT
*** Bug 74683 has been marked as a duplicate of this bug. ***
Comment 14 Krzysztof Daniel CLA 2007-11-19 11:57:39 EST
I have been thinking this over.

Until now we focused on commiting change sets and trying to inform the user about such a possibility. Maybe this is wrong - since there was not a lot of community attention here, we could just provide two additional checkbox in preferences, on the CVS tab:
1. Commit change sets separately
2. Skip commit wizards.
(The second option would be possible to select only when the first were checked, if both options are checked, we should display only warning).

I think this may solve problem of automagic behavior - only user that knows what he is looking for will be able to enable this functionality.

What do you think, Michael?
Comment 15 Michael Valenta CLA 2007-11-19 12:07:04 EST
That sounds reasonable. I think that the second option should only skip the prompt if all the chanegs are in a change set. I think we would still want to prompt if the commit contains changes that are not in a change set. Therefore, I would change the wording of the second option to "Do not show the commit wizard for changes contained in a change set" or something along those lines.
Comment 16 Tomasz Zarna CLA 2007-11-20 10:12:58 EST
Created attachment 83334 [details]
Deluxe wizard

I like all the ideas you guys mentioned, but my favorite is the one by Michael in comment 3 - a deluxe solution. It's not that I like to make things harder, actually I believe that introducing the wizard will help the user to understand and use it as expected. Anyway, I hope I managed to visualize what Michael meant in his comment (see the attached image). Here are my suggestion and things I would like to see in that wizard:

* There are two options at the bottom of the dialog: "use the comment for all resource" and "use comments from the change sets"
* Initially the first option is selected and the comment area shows combined comments from the change sets (as it is now)
* Selecting the second option and clicking commit results in a commit performed for each change set. If there are some unassigned resources a warning should be displayed saying that they don't have a comment. To set a comment the user selects <unassigned> set and enters a desired comment. We could even automatically set the selection on that set and focus on the comment area when the option is selected

What's your opinion Krzysztof? Do you think the wizard of this form is useful?

BTW, the latest patch is out-dated, but is not a surprise as it's quite old. Even guessing the fuzz factor doesn't help much here :)
Comment 17 Krzysztof Daniel CLA 2007-11-20 10:51:11 EST
After short talk with Tomasz we agreed that we should mix both solutions:

Unassigned resources belongs to <Unassigned> Change Set, as in Sync View.

By default deluxe commit dialog appears. User may choose if he wants commit change sets separately or with one aggregated comment (default). 

We put a link to preferences on the dialog, so the user will be able to disable commit wizard, and commit change sets silently.

If any problem encountered (unassigned resources) we display delux dialog. 
Comment 18 Tomasz Zarna CLA 2007-11-20 10:56:02 EST
Sounds great, doesn't it? :)
Comment 19 Krzysztof Daniel CLA 2007-11-23 09:30:08 EST
I have managed to create dialog (early beta ;-) ) that shows only if we have selected at least one change set, but now I am trying to embedd there Change Set tree. I need some hints how to do that.
Comment 20 Tomasz Zarna CLA 2007-11-23 10:19:00 EST
Take a look at ChangeSetContentProvider[1]. Except the Sync View you can also see a tree viewer with change sets in the Create Patch wizard[2] (you will need to turn the "Show Change Set" option on).

[1] org.eclipse.team.internal.ccvs.ui.mappings.ChangeSetContentProvider
[2] org.eclipse.team.internal.ccvs.ui.wizards.GenerateDiffFileWizard.LocationPage
Comment 21 Will Hains CLA 2007-11-23 23:08:34 EST
My $0.02: It's starting to sound a bit over-complicated. My favourite suggestion so far was comment #14 - ie. that there is an option added in preferences to enable silent committing of change sets, which is off by default, so unless you want this nothing changes.

Regarding the issue of selecting a mixture of change sets and unassigned changes, if the option is turned on in preferences, it should silently commit what it can, and prompt as normal for the unassigned changes only.

Which leads me to another use-case: it's going to be common to select ALL the change sets and commit, so there should be a button on the Synchronize view to "Commit All", which has the same effect.

I think that would cover everything... Is there a corner case or use-case I'm missing here?
Comment 22 Tomasz Zarna CLA 2007-11-26 04:25:09 EST
(In reply to comment #21)

Will, as Christopher stated in comment 17 we will use both ideas: add a preference to hide the commit dialog when possible and at the same time extend the commit dialog to work smoother with change sets.

As you suggested, the preference would be turned off be default, but if on the dialog will pop-up only when committing some unassigned changes. The only difference is that with the dialog you will be able to alter comments for any changes sets too (not only for the unassigned changes). What do you think? Is that what you would like to get?

> Which leads me to another use-case: it's going to be common to select ALL the
> change sets and commit, so there should be a button on the Synchronize view to
> "Commit All", which has the same effect.

There is a button called "Commit All Outgoing Changes..." in the Sync View toolbar. It should do the job.
Comment 23 Szymon Brandys CLA 2007-11-26 07:44:01 EST
Kuba, work on this issue please.
Comment 24 Will Hains CLA 2007-11-26 11:45:09 EST
(In reply to comment #22)

When you put it like that it sounds OK.
I think the killer use-case which must be covered is:
1. Assign all outgoing changes to change sets.
2. Click the Commit All button.
3. That's it. Eclipse starts silently committing all changes according to their change sets in the background. Aaaah! :-)

This improvement alone will get me to the pub a full 45 seconds sooner! :-)

Thank you to all for giving it such careful thought.
Comment 25 Szymon Brandys CLA 2008-04-16 04:29:05 EDT
Postponing to 3.5.
Comment 26 Szymon Brandys CLA 2009-03-06 06:08:45 EST
We haven't investigated the issue during 3.5 and we don't plan to address it now. I'm removing the target milestone but we'll set more realistic one when we have feedback from anyone interested in fixing this.
Comment 27 Eclipse Webmaster CLA 2019-09-06 16:04:22 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.