Thread (1 message) 1 message, 1 author, 2016-08-23

Re: [PATCH v2 5/7] xdl_change_compact(): introduce the concept of a change group

From: Junio C Hamano <hidden>
Date: 2016-08-23 21:36:03

Michael Haggerty [off-list ref] writes:
The idea of xdl_change_compact() is fairly simple:

* Proceed through groups of changed lines in the file to be compacted,
  keeping track of the corresponding location in the "other" file.

* If possible, slide the group up and down to try to give the most
  aesthetically pleasing diff. Whenever it is slid, the current location
  in the other file needs to be adjusted.

But these simple concepts are obfuscated by a lot of index handling that
is written in terse, subtle, and varied patterns. I found it very hard
to convince myself that the function was correct.

So introduce a "struct group" that represents a group of changed lines
in a file. Add some functions that perform elementary operations on
groups:

* Initialize a group to the first group in a file
* Move to the next or previous group in a file
* Slide a group up or down

Even though the resulting code is longer, I think it is easier to
understand and review.
Yup.  The important thing is that the length of the core logic of
sliding up and down becomes easier to read, because it shrinks; the
mechanics of sliding up and down may need more lines with boilderplate,
but they are isolated "do one thing and do it well" helpers.

Nice.

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help