Bugzilla – Full Text Bug Listing
| Summary: | Make revision selection dialog more intuitive (automatically include lesser revision changes into the selected range) | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Technology] Subversive | Reporter: | Christian Herold <christian.herold> | ||||
| Component: | UI | Assignee: | Igor Burilo <igor.burilo> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | major | ||||||
| Priority: | P3 | CC: | a.gurov, alexander.malic, alexei.goncharov, christian.herold, jtk499, michael.mess, peters | ||||
| Version: | 0.7 | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Christian Herold
I can confirm that this happened also for me. subversive has severe problems. merging is the most dangerous. why isn't this bug fixed? it's already 8 months old? I still hope this bug will be fixed! Version 0.7.9.I20110419-1700 contains this bug. This is a very nasty bug, because you don't see any error. You just didn’t merged all revisions you selected. Please fix it! We are aware of this bug for quite a while. The only workaround is to transform the given range,
e.g. {{5004-5002}} by hand into {{5002,5003,5004}}. Since we very seldom merge larger
range, this is annoying but viable.
This conversion has become some sort of subconscious habit in the meanwhile, but is anyway a
very serious bug which would mainly hit new-comers.
As you are normally a bit more afraid of merging this add extra FUD to this operation.
So: Igor, Alexander, Alexei -- what could be done here?
Hello All, Actually, I quite don't understand the meaning of the problem? Is the only problem is that revisions in the edit box listed from greater to lesser numbers? Why I'm asking such a strange question? It's because this is the only thing I could see here, I'm using merge several times per week and each time specifying revision ranges, and never even once merge failed me (I've never tried to reorder revisions manually because there is no meaning to it). And if you need to revert changes there is "Reverse Merge" check box (it is equivalent to specifying revisions from lesser to greater in the edit box). Is there something wrong in what I'm doing while merging (and that is why I have no issues with it? :-) ) ? Or is the actual problem is that the people like to specify revisions manually and they do it from lesser to greater numbers typically? If that is the case then it is not the problem to reverse the order automatically, although I can't say it is a bug, after all it is the order which is required by SVN API :) . Hi Alexander, thanks for shedding some light on this. Personally, my experience is as follows: Some while ago there were several issues with Subversive merging, including showing conflict were no conflicts are etc. These bugs are resolved in the meanwhile. In addition, at this time we made the experience that the merge left out some changes (for whatever reason, I do not remember). Looking at the reverse revision which looks strange to people with less SVN experience (or just prefer the -c option in favor of -r), one of the (false) guesses was that the reverse range could be the culprit. This ended in the habit of transformation all ranges into single revisions in increasing order. So reverse order is not the only issue the people experienced, but it looks unnatural. I have no idea whether there could done something about it except always reorder it (and let the Subversive code do the Right Thing finally). This would also help situations were people specify revisions by hand. Here they would naturally using increasing ranges. Finally, I would propose that the order of ranges is always increasing and that decreasing ranges are "normalized" to increasing ones. The ranges selected by the browser should also be displayed in this way. If the user specifies "Reverse merge", than the ranges are reversed by the backend as appropriate. In this way, the UI gets more stable and less dependent on the order the people tend to specify. The only advantage you will use it the ability to mix increasing and decreasing ranges, but this sounds to me as a very rare usage. Anyway, I trust your experience and from now on I will encourage of revision ranges as returned from the revision browser. What do you think about these ideas? Cheers, Jörg Just another note: Looking at the description it seems that the **Preview mode** displays bc. svn merge -r 5002:5004 "http://svnhost" "/Path/project" --dry-run --username "user" if "5004-5002" is specified. Is the different syntax (":" instead of "-") confusing here? i tink that's the problem. you should create a patch and send it to the subversive guys! (In reply to comment #9) > i tink that's the problem. you should create a patch and send it to the > subversive guys! Sorry, but I am not in the position to create such a patch due lack of time and in-depth knowledge. But lets wait for Alexanders comments. Thank you for detailed explanations. So, in the end it looks like it all is just a misunderstanding that is constantly created due to ambiguous nature of specifying revisions. If that so, then we could just do as Jörg said and always rearrange revisions and revision ranges before any further steps (like "Reverse Merge" etc.) > In addition, at this time we made the experience that the merge left out some > changes (for whatever reason, I do not > remember). There is a small trick to it, lets say we need to merge 1 revision with number 12345 into our code. What do we do with a revision range in that case? We should specify 12345-12344 as the revision range, because changes could be seen as a difference between two revisions, right? Then we should remember well this moment: when we need to merge revision №12345, 12346, 12347 and 12348 into our code we should specify 12348-12344 as our revision range, but not the 12348-12345, otherwise there will be: > left out some changes > What do you think about these ideas? I think they're just what we need to solve all the misunderstandings. :) >if "5004-5002" is specified. Is the different syntax (":" instead of "-") confusing here? I don't think so, after all using '-' character is the most typical and standard way of specifying ranges in math, datasheets etc. Command line is command line and people are people, lets leave it at this. >i tink that's the problem. you should create a patch and send it to the subversive guys! Regarding patches - there is no need to do this, it's just that sometimes it is hard to place yourself into the software user position in order to understand the problem, when you don't see any real issue. That is the real nature of the "usability" term. :( (In reply to comment #4) > We are aware of this bug for quite a while. The only workaround is to transform > the given range, > e.g. {{5004-5002}} by hand into {{5002,5003,5004}}. Since we very seldom merge > larger > range, this is annoying but viable. Oh, and there it is, the reason is described in my message earlier. It is really not an issue but still could be a little confusing and so, I'm not sure what should we do with this. When we choose revisions in the revision selection dialog - we can hack selected range and decrement lower bounds of every range by one, but when we enter this kind of information manually - there is no meaning in changing it. What if I meant exactly what I wrote in the edit box (API uses ranges as I mentioned before)? Where should I (and should I at all?) change those user-defined ranges? I don't think so. And in the end the only part that could be improved there is automatic reordering of the specified revisions and revision ranges. Sorry for the confusion ..
I think there is no problem with the ordering or the syntax.
It's the conversion from selecting Changesets (or revisions) in the GUI and transforming this to SVN changesets.
If I made a change (based on revision 5001) and commit this change to subversion with revision 5002 the changes are made from revision 5001 to 5001.
To merge this changeset a have to use svn merge -r 5001:5002 (or svn -c 5002).
So I will get all changes made between those revision to me code.
When using the eclipse merge GUI a get a list with Revision, Date and Comment.
5001 ("base")
5002 ("my new change")
When selecting only 5002 in the revision list the number 5002 is filled in the "Revisions:" dialog box.
Running preview show the corresponding svn command: svn merge -r 5001:5002
When you select more than one revision (5001, 5003, 5005) but skipping one revision between each selected the GUI uses 5001, 5003, 5005 and SVN uses:
svn merge -r 5000:5001 -r 5002:5003 -r 5004:5005
The changes in revisions 5002,5004 won't be used.
That’s ok.
But when you select more than 2 revision (a whole range, without skipping: 5001,5002,5003,5004,5005) the GUI transforms this revision to a revision-range 5005-5001 (the ordering is not the problem).
But this is used with svn as:
svn merge -r 5001:5005
And this is not what I thought it should be, because it does not contain the changes in revision 5001 !
I think the solution could be to use single revisions in the GUI and not transform this to ranges (5001,5002,5003,5004,5005) or calculate the minimum revision "-1"
a)
Revisions: 5001,5002,5003,5004,5005
svn merge -r 5000:5001 -r 5001:5002 -r 5002:5003 -r 5003:5004 -r 5004:5005
b)
Revisions: 5000-5005
svn merge -r 5000:5005
the second solution would be nicer when using huge ranges...
So:
I can't see any problem with ordering.
The problem is based on conversion of revision ranges.
This are the lines from SVN doc:
svn merge [[-c M]... | [-r N:M]...] [SOURCE[@REV] [WCPATH]]
-c M is equivalent to -r <M-1>:M, and -c -M does the reverse: -r M:<M-1>.
>This are the lines from SVN doc:
>svn merge [[-c M]... | [-r N:M]...] [SOURCE[@REV] [WCPATH]]
>-c M is equivalent to -r <M-1>:M, and -c -M does the reverse: -r M:<M-1>.
And this is how it works currently in Subversive UI, right?
Then about this part the only thing I could suggest is to create different ranges when revision selection dialog is used, because what is entered manually should not be changed, UI can't suggest user thoughts in such a matters.
So, to say it simple, we can do that if revisions 5001-5005 are selected in the revision selection dialog, then it will be converted into the 5005-5000 range. Will this solve the misunderstanding?
(In reply to comment #13) > Sorry for the confusion .. > [...] Thanks for the clear examples. > But when you select more than 2 revision (a whole range, without skipping: > 5001,5002,5003,5004,5005) the GUI transforms this revision to a revision-range > 5005-5001 (the ordering is not the problem). > But this is used with svn as: > svn merge -r 5001:5005 > > And this is not what I thought it should be, because it does not contain the > changes in revision 5001 ! Alexander, here I would expect that when the GUI displays "5005-5001", the backend actually issues: svn merge -r 5000:5005 If this is not the case, then this is a bug. > [...] > b) > Revisions: 5000-5005 > svn merge -r 5000:5005 > > the second solution would be nicer when using huge ranges... I would leave the "-1" to the backend: b') Revisions: 5001-5005 svn merge -r 5000:5005 since it is more intuitive to the user. The ordering of ranges should not matter, i.e. both ordering give the same result in the back end: b1) Revisions: 5005-5001 svn merge -r 5000:5005 b2) Revisions: 5005-5001 svn merge -r 5000:5005 but with "Reverse merge" enabled: b3) Revisions: 5005-5001 svn merge -r 5005:5000 b4) Revisions: 5005-5001 svn merge -r 5005:5000 In this way, the user could enter revisions manually or select them via browser and the numbers selected in browser reappear 1:1 in the GUI. The concrete "svn" command syntax does not matter here because most GUI users (including me) tend to avoid it. What do you both think? >Alexander, here I would expect that when the GUI displays "5005-5001", the backend >actually issues: >svn merge -r 5000:5005 > >If this is not the case, then this is a bug. I don't think so. When I enter manually revisions and I want changes only between those revisions should I in your case increment lesser revision by 1 in my mind? This will be so wrong, at least currently it works the same way as Subversion doc says. And that is why I've said earlier: >So, to say it simple, we can do that if revisions 5001-5005 are selected in the revision selection dialog, then it will be converted into the 5005-5000 range. Then in that case if you select revisions in the revision selection dialog, you will get changes from these revisions and when you enter revision ranges manually you will get what SVN docs says you. That is what I could agree with. (In reply to comment #16) > I don't think so. When I enter manually revisions and I want changes only > between those revisions should I in your case increment lesser revision by 1 in > my mind? This will be so wrong, at least currently it works the same way as > Subversion doc says. And that is why I've said earlier: > > >So, to say it simple, we can do that if revisions 5001-5005 are selected in the > revision selection dialog, then it will be converted into the 5005-5000 range. > Then in that case if you select revisions in the revision selection dialog, you > will get changes from these revisions and when you enter revision ranges > manually you will get what SVN docs says you. That is what I could agree with. OK, then the other way round :-) You like to see the svn command line syntax in the GUI Revisions field, but with ":" replaced by "-". And you suggest that the GUI Browser shall return this syntax by e.g. filling "5000-5005" if revisions 5001-5005 are selected in the GUI Browser. That is consistent. But how about reverse merges? What does the tick box "Reversed merge" do? Does it apply to the revisions selected by the GUI browser only? Then it should be part of the browser, not of the base dialog. Or does it also apply to the manually entered versions? Then the order would not matter and could also be "5000-5005" in your above example. I would expect that the GUI browser then always returns incrementing ranges. It also applied to the manually entered ones. 'Reverse Merge' - is simply removing selected changes from your code and so, the current place is quite right. *** Bug 324936 has been marked as a duplicate of this bug. *** (In reply to comment #18) > It also applied to the manually entered ones. 'Reverse Merge' - is simply > removing selected changes from your code and so, the current place is quite > right. OK, so the only open issue is that the GUI Browser should fill in the text "5005-5005" if revisions 5001, 5002, 5003, 5004 and 5005 are selected? With regard to increasing ranges: The svn command line docs says that "-c 5001" is equivalent to "-r 5000:5001" so here is the range increasing. I would like to also see that in the GUI. Christian, do you agree in recent points? >OK, so the only open issue is that the GUI Browser should fill in the text "5005-5005" if revisions 5001, 5002, 5003, 5004 and 5005 are selected? A little correction: 5000-5005. >The svn command line docs says that "-c 5001" is equivalent to "-r 5000:5001" so here is the range increasing. I would like to also see that in the GUI. That could be done too if it will be easier to use in the end. OK, I am fine with the results so far. Болшое спасибо и хороший конец недели :-) Jörg Nice weekend to you too, Jörg. And it is me, who should be thanking you for all your efforts. So, thank you very much! < on more serious note :) > Done. Hi I'm still late with my answer... I'm very fine with the solution. 1. The manual entries should be consistant with svn command-line. 2. changes will be done in the GUI only (decrease lower bound). Thanks a lot Tested with new version 0.7.9.I20110523-1700 and looks fine. Alexander, the first tests look good. Today or tomorrow I have a larger merge with many, non-continuous revisions. I will report the result to you. Quick question with regard to the start of ranges: A colleague mentioned that quite a while ago it often happened, that if a range of revisions was selected, the first revision was omitted. Therefore, she stopped using ranges. Do you remember of any related bug to this behaviour? When was it corrected? (In reply to comment #27) > Alexander, the first tests look good. > > Today or tomorrow I have a larger merge with many, non-continuous revisions. I > will report the result to you. > > Quick question with regard to the start of ranges: > > A colleague mentioned that quite a while ago it often happened, that if a range > of revisions was selected, > the first revision was omitted. Therefore, she stopped using ranges. > > Do you remember of any related bug to this behaviour? When was it corrected? That was not a bug, that was normal SVN behaviour - difference between revisions. It's just like in math: 5-4 = 1 and not 2, this way difference between revision 5001 and 5000 is the revision 5001 content only. It could be confusing while selecting revisions in the Revision Selection dialog, that is why I have changed this dialog behaviour to include all selected revisions changes, but when you enter revisions manually it still will be the same behaviour as with SVN: if you want to merge changes from 5001 and 5000, you should write in the edit box the 4999-5001 range and not the 5000-5001 one. Selection of 5000 and 5001 revisions in the Revision Selection dialog will produce 4999-5001 range automatically. Not sure whether she used the browser or entered the ranges manually. In the latter case, your comment applies. Created attachment 196542 [details]
Revision browser for week returns 14870-14972
While the release id are non-continuous,
returned range is 14870-14972.
Is this expected? I would expect a comma separated list.
At least, I guess it works as it should.
(In reply to comment #30) > Created attachment 196542 [details] > Revision browser for week returns 14870-14972 > > While the release id are non-continuous, > returned range is 14870-14972. > > Is this expected? I would expect a comma separated list. > > At least, I guess it works as it should. Yes, it works as expected. There is no reason to generate comma-separated list because for the selected URL only those revisions, shown in the dialog, exists anyway. And so, there will be no unneeded changes merged into your code. Comma-separated list of revisions and (or) revision ranges will be created only in case when you select non-contiguous revisions in the Revision Selection dialog for the specified URL. Will you also update the online help section "SVN Merge Dialog" with the results from this discussion? (In reply to comment #32) I've added corresponding task: bug #347143. |