Re: [PATCH v2 06/37] diff: rename 'this' variables
- From
- Brandon Williams <bmwill@google.com>
- Date
- Feb 14, 2018, 21:24 UTC
- Message-ID
- <20180214212438.GC139458@google.com>
- In-Reply-To
- <xmqqh8qjmfq5.fsf@gitster-ct.c.googlers.com>
On 02/14, Junio C Hamano wrote:
Show 16 quoted lines
> Brandon Williams <bmwill@google.com> 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 <bmwill@google.com> > > --- > > 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.
Show 36 quoted lines
>
> > 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