Re: [RFC PATCH v5 2/4] add -p: select modified lines correctly
- From
- Phillip Wood <phillip.wood@talktalk.net>
- Date
- Jul 27, 2018, 10:19 UTC
- Message-ID
- <2e78e077-1801-9c92-6b53-badf588fe262@talktalk.net>
- In-Reply-To
- <xmqqmuudpzfl.fsf@gitster-ct.c.googlers.com>
Hi Junio, thanks for the comments
On 26/07/18 20:30, Junio C Hamano wrote:
Show 35 quoted lines
> Phillip Wood <phillip.wood@talktalk.net> writes: > > An interesting problem you are solving ;-) > >> For example given the hunk >> 1 -* a longer description of the >> 2 - first item >> 3 -* second >> 4 -* third >> 5 +* first >> 6 + second item >> 7 +* the third item >> >> If the user selects 1,2,4–5,7 then we should generate >> -* a longer description of the >> - first item >> +* first >> * second >> -* third >> +* the third item > > I understood this example as "3 that is removal and 6 that is > addition are excluded---we consider that these two lines (one in the > pre-image and the other in the post-image) are _matching" As we> are excluding a deletion, it becomes the common context line, and > any removal or addition that appear before that must stay to happen > before the common context line (i.e. removal of 1 and 2, and > addition of 5, both precede common context line "second") and any > removal or addition that appear after that must stay after the > common context (i.e. removal of "third" and addition of "the third > item" come after "second"). > > But then it is not clear to me what you mean by "group" below. What > groups does the above example have? Ones before the retained > "second" (i.e. removal 1, 2, 4 and addition 5) form one group and > ones after it (i.e. removal 4 and addition 7) form another group?
The code actually looks at the lines that are selected rather than omitted. So in the example above it groups them as [1,2] (because they are contiguous), [4],[5] (these are split because one is an insertion and one a deletion) and [7]. It then sees that there are two groups of deletions ([1,2],[4]) and two groups of insertions ([5],[7]) and so pairs up the deletions in [12] with the insertion in [5] and likewise with [4] and [7]. Lines 3 and 6 are never explicitly paired, although they basically behave as if they were. One the insertions are all paired up it walks over the list and creates a new hunk where the paired insertions come immediately after their corresponding deletions, unselected deletions are converted to context lines and unselected additions are dropped.
Show 6 quoted lines
> >> Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > > Is this fixing any bug? I usually see "Reported-by" only for a > bugfix patch but this seems to be adding a new feature (and lack of > feature is usually not a bug).
I guess I meant that the previous series was effectively buggy as it would give the wrong result for modified lines. I wanted to acknowledge that Ævar spent some time testing it and pointed that out.
Best Wishes
Phillip