Subject: Re: [PATCH v2 06/37] diff: rename 'this' variables

From: Brandon Williams <[email-masked]>
Date: Wed, 14 Feb 2018 13:24:38 -0800

On 02/14, Junio C Hamano wrote:
> Brandon Williams <[email-masked]> writes:
> 
> > Rename C++ keyword in order to bring the codebase closer to being able
> > to be compiled with a C++ compiler.
> >
> > Signed-off-by: Brandon Williams <[email-masked]>
> > ---
> 
> The patch is not as bad as renaming "this" and leaving "that" behind
> but in the original, "this" and "this_dir" were treated as a pair.
> "this" was a score for a single item in the directory, "this_dir"
> was the sum of these scores for entries in the directory.
> 
> So renaming "this" to "sum" and leaving "this_dir" as-is looks like
> readability regression.  Perhaps replace "this_dir" with "sum_changes"
> and "this" with "changes" instead, or something like that?

100% agree.  I tried to spend time on each of these changes to come up
with a meaningful name, but sometimes wasn't able to understand the
section of code (with the limited context I had with some areas) and
wasn't able to come up with the best name.  I'll change this.

> 
> >  diff.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/diff.c b/diff.c
> > index 0a9a0cdf1..d682d0d1f 100644
> > --- a/diff.c
> > +++ b/diff.c
> > @@ -2601,7 +2601,7 @@ static long gather_dirstat(struct diff_options *opt, struct dirstat_dir *dir,
> >  	while (dir->nr) {
> >  		struct dirstat_file *f = dir->files;
> >  		int namelen = strlen(f->name);
> > -		unsigned long this;
> > +		unsigned long sum;
> >  		char *slash;
> >  
> >  		if (namelen < baselen)
> > @@ -2611,15 +2611,15 @@ static long gather_dirstat(struct diff_options *opt, struct dirstat_dir *dir,
> >  		slash = strchr(f->name + baselen, '/');
> >  		if (slash) {
> >  			int newbaselen = slash + 1 - f->name;
> > -			this = gather_dirstat(opt, dir, changed, f->name, newbaselen);
> > +			sum = gather_dirstat(opt, dir, changed, f->name, newbaselen);
> >  			sources++;
> >  		} else {
> > -			this = f->changed;
> > +			sum = f->changed;
> >  			dir->files++;
> >  			dir->nr--;
> >  			sources += 2;
> >  		}
> > -		this_dir += this;
> > +		this_dir += sum;
> >  	}
> >  
> >  	/*

-- 
Brandon Williams