Community
Participate
Working Groups
I was recently trying to apply a patch where there had been a minor change in the context lines. The auto-merge failed (which is reasonable). However, when I tried to merge the lines in the hunk editor, it did not assist me in finding the correct insertion point. Enhancement request: Add UI to the hunk compare editor to assist with locating places in the file where the context lines "almost" match. One possible implementation: - Provide a "search for insertion point" button. It can find the insertion point that results in the biggest longest common subsequence between the file's lines and the context lines. There could also be a "find next" button that looks for the next-longest LCS. Or perhaps the editor could just run this algorithm on startup and suggest the result as the insertion point. - Once a suggested insertion point is found, decorate the context lines to indicate which bits match and which bits don't.
Stefan, I agree that this would be a nice feature. As you've probably noticed I did release a fix for the the bug 199846, which resulted in a possibility to apply a hunk even though some of it's context lines don't match. This is provided by the fuzz factor, which now allows to ignore specified number of context lines. If I understood you correctly, this could be the answer for finding the insertion point issue. The other thing you pointed out is the fact that we should somehow decorate the context lines. If you confirm that the fuzz factor solves the issue described in comment 0, I hope you will also agree that bug 205761 will address the decorating issue, too.
This would be a really handy feature. Dealing with non-matching hunks is unpleasant. Any tool support that would improve the user's experience would help.
Created attachment 84197 [details] Mockup After I did a second look at Stefan's comment I think I might have misunderstood his intention. A few minutes later, I came across bug 190844, where in the first bullet Stefan is sharing with us his idea of how to deal with insertion points. Then I decided to create a mockup which will help me to understand how could the UI for all these features look like. I put there 3 things: 1) "move up/down" buttons 2) indicator of the suggested insertion point on the left 3) "find next/prev LCS" buttons All of them would be available only when there is a conflicting change. 1) would move the suggested insertion point up and down the file. 2) would show us the initial position of the insertion point and finally 3) could find the insertion point that results in the biggest longest common subsequence between the file's lines and the hunk lines. This is only a sketch, but I think it can be a good starting point for the discussion on how these features should look like and, what's more important, how what should actually do. Any comments are welcome.
I would prefer not to see more mysterious buttons on the compare editor. Would there be more passive ways of showing potential matches? I can see four cases for both the preamble and postamble of a missing hunk. Each can be in one of the following states: 1. It exists uniquely in the destination. (ie, a unique full match) 2. Multiple copies exist in the destination. (ie, a multiple full match) 3. One or more lines exist verbatim and in order in the destination. (ie, a partial match) 4. None of the lines exist in the destination. (ie, no match) In other words, a preamble may exist uniquely in the destination, but the postamble may not, or some other permutation. When the user selects a hunk, would it be possible to highlight the full and partial matches in the destination? Different colours or scores could be used to show the quality of match. If the preamble and postamble are highlighted together, the user would have a better chance of finding an entry point. I'll put together a mockup.
Thanks for taking the time to investigate this, Tomasz. The screenshot looks good. - Just an observation: I notice that the mockup seems to be based on the three-way editor rather than the hunk editor. Was that intentional? - Now that I look at it, moving the insertion point one-line-at-a-time may be a bit cumbersome. Perhaps we could also let the user move the insertion point using drag-and-drop? Some sort of grippy should probably be drawn around the insertion point to indicate that it is draggable. I have an idea as to how this might be done and will attach a mockup if I can. - I assume that the LCS buttons iterate from best insertion point to worst rather than from top of the file to bottom. Is this correct? > I would prefer not to see more mysterious buttons on the compare editor. I have an idea about this... see my (forthcoming) mockup... > I can see four cases If it were case 1 or 2 the patch would have applied you wouldn't have seen the hunk editor. You'll usually only see the hunk editor for cases 3 or 4, and I *think* both of them could be handled using the LCS algorithm I suggested.
(In reply to comment #5) > > I can see four cases > > If it were case 1 or 2 the patch would have applied you wouldn't have seen the > hunk editor. You'll usually only see the hunk editor for cases 3 or 4, and I > *think* both of them could be handled using the LCS algorithm I suggested. > The cases were: >>1. It exists uniquely in the destination. (ie, a unique full match) >>2. Multiple copies exist in the destination. (ie, a multiple full match) >>3. One or more lines exist verbatim and in order in the destination. (ie, a >>partial match) >>4. None of the lines exist in the destination. (ie, no match) That isn't the case. The preamble may match, but the postamble may not. In that case the hunk fails to match.
Created attachment 84226 [details] Mockup of partially matched hunk Here's a mockup of one way of showing partially matching preamble/postambles. The file is being patched has extra comments, preventing the entire patch from matching. 1. Each preamble/inamble/postamble is highlighted in the ancestor and "Unmatched lines" panes. I'm using different colours here to highlight the match, but other mechanisms could be used. 2. Complete matches of *ambles are decorated with three vertical lines in the margin, partial matches are shown with fewer, depending on the number of lines of the *amble that match. 3. *ambles that match are linked from the "Unmatched Lines" pane to the destination pane. The links should be decorated for perfect matches. 4. The *ambles would be highlighted modally. When the user selects a region, the surrounding ambles are highlighted.
Created attachment 84235 [details] Re-attachment of my mockup This demonstrates another approach to controlling the insertion position. (Rather than adding toolbar buttons, decorate the insertion position)
> Here's a mockup of one way of showing partially matching preamble/postambles. I'm not sure I like the number of colours used in that mockup, but I do like the fact that matching context lines are shown.
(In reply to comment #9) > > Here's a mockup of one way of showing partially matching preamble/postambles. > I'm not sure I like the number of colours used in that mockup, but I do like > the fact that matching context lines are shown. The colours are iffy. I want to show where the chunk probably matches. The current tooling doesn't provide any indication of where the *ambles fit, which means the user has to do much more work. Another possibility would be to highlight the currently selected match, and only show the *amble on either side - but that wouldn't allow the user to use the context of other *ambles to move the hunk around.
Just to be clear, Tomasz, I think your mockup was fine. I'm only intending my comments as suggestions, not as criticism. Evan: I liked the idea of showing matches in the context lines... but why not make them all the same color and skip the vertical bars? I don't really see the need to show such a strong affordance indicating the difference between full and partial matches.
I wasn't too keen on the large up/down control in Stefan's mockup at first. I thought that a small box where your cursor would turn into drag up/down arrows would be enough. Like the anchors on the corners of images in graphic software, on the crop tool. But then I thought, that would only be good for the single-line situation as presented. I'm not sure how nice it would be for larger hunks. In Stefan's mock, would that control grow to be the size of the hunk? Point is: I'm all for "Less is more" as long as its useable. Secondly, how far up and down would you be able to move the insertion point? If there are multiple conflicts, would you only be able to move the insertion point between its neighbour conflicts? I don't think I've ever seen the compare editor to moves across conflicts, so that lines cross in the two window panes. It's always an deletion and addition in that case.
I had a bit of a discussion with Stefan and Evan about this. I think it would be good if we identified some different failed hunk scenarios to test some of these ideas against. Here are a few scenarios that came up in our discussion or are presented in the mockups. Scenario 1. The context lines are unmodified but lines have been inserted between context lines. This is the case that appears in Evan's mockup. Scenario 2: The hunk contains only additions (i.e. nothing gets removed from the target). This is the nature of both Evan's and Stefan's mockup. Scenario 3: The hunk contains only deletions. In a combined scenario 1 and 3, we may be able to highlight a region in the left pane that is a good candidate for deletion. Scenario 4: The hunk contains a replace where the target also has changes (i.e. a conflicting change) In a combined scenario 1 and 2 situation, having a good guess at an insertion point combined with Stafan's movable insertion point would be helpful. I'm not sure how helpful it would be for scenario 3 and 4. One though that can up was that if we could identify the region in the left pane that corresponded to the contents of the hunk and run the standard 3-way compare using only that region as the left side, we could get a presentation (and copy actions) that were similar to a standard compare. The difficulty is identifying that region in an efficient and reliable manner.
(In reply to comment #13) > I had a bit of a discussion with Stefan and Evan about this. > ... I'd like to see the compare editor show matching context lines. Even if only a subset could be displayed, it provides a useful hint to the user. This would fall under all of the scenarios that Michael mentioned. It may provide a reasonable starting place for UI work.
(In reply to comment #5) > - Just an observation: I notice that the mockup seems to be based on the > three-way editor rather than the hunk editor. Was that intentional? No. I'm sorry, I was to lazy to make a new screen show of the hunk editor, but I know you got the point. > [...] Perhaps we could also let the user move the insertion point > using drag-and-drop? Some sort of grippy should probably be drawn around the > insertion point to indicate that it is draggable. [...] Sounds like a great idea. If I could just add one thing: instead of the grippy-thing you included in your mockup I would add the Alt+Drag mechanism. The hunk being drag can still be decorated in some way. > - I assume that the LCS buttons iterate from best insertion point to worst > rather than from top of the file to bottom. Is this correct? Yup, you're reading my mind. (In reply to comment #11) > Just to be clear, Tomasz, I think your mockup was fine. I'm only intending my > comments as suggestions, not as criticism. I'm always happy to hear any comments even if it's criticism. At least I know that somebody is interested in what I'm doing. (In reply to comment #12) > If there are multiple conflicts, would you only be able to move the insertion point > between its neighbour conflicts? I don't think I've ever seen the compare > editor to moves across conflicts, so that lines cross in the two window panes. > It's always an deletion and addition in that case. Agree. There might be some cases where changing the order of hunks would find matches that wouldn't otherwise be found. However, IMO we shouldn't allow such modifications, at least not by default. After making a couple of such changes we can end up with the hunk editor being completely unreadable. The only solution that comes to my mind at this moment is that we could allow to preview individual hunks (as it can be done now for conflicting hunks). Eventually, when we decide to put hunks out of order the fact needs to be indicated to the user - see bug 205789. (In reply to comment #14) > I'd like to see the compare editor show matching context lines. Even if only a > subset could be displayed, it provides a useful hint to the user. I've been thinking about the same thing. A way to see context lines (no matter if they are matching or not) of a hunk would be extremely. This could be combined with a solution for bug 205761. I'll try to prepare a mockup that gathers all your ideas (the ones I like the most :).
Created attachment 84520 [details] Three ways of emphasizing context lines Mockup that shows three possible ways of emphasizing context lines in the hunk editor: 1) dimming lines which are not part of the patch 2) lines which are part of the patch are in bold 3) highlihgt context lines of a hunk using different colors (red color indicate that a line didn't match)
Created attachment 84523 [details] A way to preview content of a patch #1 The idea came to my mind when working with a graphic editor and making other mockups. This is an analogy with the layer concept from graphic editors when you can hide/show part of an image.
This is a different way of previewing content of a patch based on tooltips. This time it's combined with bolded lines for a patch (see comment 16, 2)). The idea basically work the same way as tooltips for javadocs in Java classes.
Created attachment 84525 [details] A way to preview content of a patch #2 Sorry, forgot about the mockup itself.
(In reply to comment #16) > Three ways of emphasizing context lines ... > 1) dimming lines which are not part of the patch > 2) lines which are part of the patch are in bold > 3) highlihgt context lines of a hunk using different colors (red color indicate > that a line didn't match) I like #2. Highlighting the context lines that didn't match with red makes it look like a conflict in the content itself. Which is somewhat confusing. (In reply to comment #17) > The idea came to my mind when working with a graphic editor and making other > mockups. This is an analogy with the layer concept from graphic editors when > you can hide/show part of an image. I don't entirely understand. Is the button showing a modified line in the hunk pane? If so, it probably wouldn't aid my understanding, but it may be helpful to other people. (In reply to comment #18) > This is a different way of previewing content of a patch based on tooltips. > This time it's combined with bolded lines for a patch (see comment 16, 2)). The > idea basically work the same way as tooltips for javadocs in Java classes. Using a tool-tip to show the patch is fine, but I think the user would benefit more from seeing what the line decoration means. Each line could have a tool-tip stating "Line matches in patch and destination"/"Line missing in destination"/"Line modified in destination", etc. One of the issues I find confusing with the current conflict editor is that I don't know what the colours mean. Having them explained with a tooltip would be helpful.
(In reply to comment #16) > Three ways of emphasizing context lines ... > 1) dimming lines which are not part of the patch > 2) lines which are part of the patch are in bold > 3) highlihgt context lines of a hunk using different colors (red color indicate > that a line didn't match) I liked 1, since in compare editors I'm already thinking about differences and so reducing visibility of things I want to ignore anyway seems right to me. I disliked 2. I find too much bold too close together harder to read. I usually prefer uses of bold to be a single word/line in the midst of non-bold. Basically, for things I need to scan, not read. I would be content with 3 if it wasn't for the red, but that could be any colour. If it were a different colour, like an off-off-yellow, then I think I would like it better than 1, since it provides more information than 1. So long as it is easy to tell apart conflicts from modified context lines. (In reply to comment #17) > The idea came to my mind when working with a graphic editor and making other > mockups. This is an analogy with the layer concept from graphic editors when > you can hide/show part of an image. I also don't understand exactly. You're saying it toggles the right-side pane to preview the content of the patch? I'd have to see it in action, but if there's too many lines inserted or deleted and having text shift around to accomodate them, then I don't think I would use it.
(In reply to comment #21) > (In reply to comment #16) > > Three ways of emphasizing context lines > ... > > 1) dimming lines which are not part of the patch > > 2) lines which are part of the patch are in bold > > 3) highlihgt context lines of a hunk using different colors Andrew raises a good point. When we're showing context lines, the user cares about what matches. If a context line has been modified, it no longer matches, so we can leave it undecorated. The user should be able to figure out what it means. I'm in favour of either dimming lines outside the patch (option 1), or highlighting matching context lines with a simple background (option 3, without highlighting conflicting context lines).
(In reply to comment #21) > I also don't understand exactly. You're saying it toggles the right-side pane > to preview the content of the patch? I'd have to see it in action, but if > there's too many lines inserted or deleted and having text shift around to > accomodate them, then I don't think I would use it. I can't argue with that. Just from your comments I can see that the idea isn't the most intuitive one. Let just forget about it. (In reply to comment #22) > If a context line has been modified, it no longer matches, so we can leave it undecorated. > The user should be able to figure out what it means. I'm afraid I'm not so sure about that. Having in mind that the Compare Editor uses color to decorate differences between changes I would use the same metaphor for the Hunk Editor (ie I would leave fragments that match undecorated). However, I do understand why highlighting the context lines that didn't match with red can be found confusing. What about using a different color? Maybe adding a special pane with the current hunk would help here? The Hunk Preview I'm thinking about could be turn on and off. I would place it next to the Patch Content pane or as a third column on the right side of the After Patch pane. This way the user is not overwhelmed with information and can open the details pane only when needed. I can create a mockup if you wish. As an alternative to a tool-tip we could also use the status line. I'm not sure if it's a common practice to add the status line to the dialog but there is always the status line on the workbench window. I've been also thinking about combining both above ideas, instead of adding the whole hunk preview and the status line we could create a single line preview of the selected hunk. The preview could be enhanced with an information what it means/does (removing line, matching context line, changed line and so on). Again, if you wish, I will crate a mockup in the blink of an eye.
(In reply to comment #23) > (In reply to comment #22) > > If a context line has been modified, it no longer matches, so we can leave it undecorated. > > The user should be able to figure out what it means. > > I'm afraid I'm not so sure about that. Having in mind that the Compare Editor > uses color to decorate differences between changes I would use the same > metaphor for the Hunk Editor (ie I would leave fragments that match > undecorated). However, I do understand why highlighting the context lines that > didn't match with red can be found confusing. Looking back through the comments, we all seem to agree that matching context lines should be shown. The comments from 16 onwards mostly concern appearance of matching regions. Would it be possible to create a prototype editor that would allow us to experiment with the different appearances? It sounds like significant regions include: 1. complete context matches (ie, entire context matches once), 2. duplicated complete context matches (ie, multiple instances of context), 3. context matches with changes within lines, (ie, context matches, but with a number of changes within individual lines) 4. context matches, but is out of order (ie, leading context in the patch follows the trailing context in a patch) 5. context fails to match Implicit in (3), (4), and (5) is the idea of a line with partial matches. That could be a ratio of tokens in a patch line that must be present in a target line to be considered a match. > What about using a different color? I already have a hard time remembering the difference between black-highlighted content and blue-highlighted content. Adding another colour would make things more confusing. But this is another presentation concern. =)
Please take a look here: http://polishineclipse.blogspot.com/2008/03/how-to-make-hunk-compare-editor-more.html It's a brief description of changes made so far in other dependent bugs. Any comments here or on the post are welcome.
I'm afraid I won't make it in 3.4 M7, which is in fact the last iteration for 3.4. Thus I'm moving it to 3.5.
Even though we did some work in the field of applying patches during 3.5, this particular issue was not one of them. However, I'm pretty sure there will be further work in this area during 3.6. Moving to 3.6.
What had been done in 3.6, which may be considered related to this issue, is applying patches in the Sync view (bug 236169). From now on, when sync with a patch, incoming changes can be reviewed in a regular, compare editor. This gives us a lot more options how to fix this bug... which hopefully we will find time to implement in 3.7. Any ideas on how to improve unmatched hunks handling are still welcome. PS. I will reschedule the bug for 3.7 as soon as the target milestone is available.
Moving to 3.7.
We do not plan to fix this during 3.7, removing the target milestone. Don't hesitate to ping us if this is an important bug for you or add it to our 3.7 Draft plan[1] if you think you can help us fixing it. [1] http://wiki.eclipse.org/Workspace3.7
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.
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. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. 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. -- The automated Eclipse Genie.
