Re: [PATCH 1/8] xdl_change_compact(): rename some local variables for clarity

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/04/2016 09:06 AM, Jeff King wrote:
> On Thu, Aug 04, 2016 at 12:00:29AM +0200, Michael Haggerty wrote:
> 
>> * ix -> i
>> * ixo -> io
>> * ixs -> start
>> * grpsiz -> groupsize
> 
> After your change, I immediately understand three of them. But what is
> "io"?

The (pre-existing) convention in this function is that variable names
dealing with the "other" file have a trailing "o"; e.g., (xdf, xdfo),
(rchg, rchgo). There used to also be (i, io), the indexes tracking the
current line number in the file and the other file. But I renamed "i".

At first I was just going to add a comment for variable "io", but in
trying to figure out its exact semantics I realized that this code is
still pretty hard to follow. Part of the problem is that "the line in
the other file corresponding to a line in the to-be-compacted file" is
not a well-defined concept. In fact it is *groups of lines* that
correlate with each other. So I totally refactored the function, using a

    struct group {
            long start, end;
    };

as a kind of a cursor used to iterate through the groups on both sides.
I think the result is a lot easier to read, and while refactoring I
found and fixed another bug in the pre-existing code :-O

I hope to have v2 out soon.

Michael

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]