Re: [PATCH 3/7] diff.c: do not pass diff options as keydata to hashmap

From: Jeff King <peff@peff.net>
Date: Apr 6, 2018, 20:04 UTC
Message-ID: <20180406200443.GE11450@sigill.intra.peff.net>
In-Reply-To: <20180402224854.86922-4-sbeller@google.com>

On Mon, Apr 02, 2018 at 03:48:50PM -0700, Stefan Beller wrote:

Show 6 quoted lines

> The diff options are passed to the compare function as
> 'hashmap_cmp_fn_data', which are given when the hashmaps
> are initialized.
> 
> A later patch will make use of the keydata to signal
> different settings for comparision.

I had to scratch my head here for a moment. Don't we use those options as part of the comparison?

I took the "which" to mean "the compare function", but I think you mean "we pass these diff options already when the hashmap is initialized".

Maybe something like this would be more clear:

  When we initialize the hashmap, we give it a pointer to the
  diff_options, which it then passes along to each call of the
  hashmap_cmp_fn function. There's no need to pass it a second time as
  the "keydata" parameter, and our comparison functions never look at
  keydata.

  This was a mistake left over from an earlier round of 2e2d5ac184
  (diff.c: color moved lines differently, 2017-06-30), before hashmap
  learned to pass the data pointer for us.

(I'm just guessing on the second paragraph based on a quick look at git-blame and my recollection from the time).

-Peff