From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Elijah Newren <newren@gmail.com>
Subject: Re: range-diff should suppress context-only changes?
Date: Thu, 05 Nov 2020 12:55:09 -0800 [thread overview]
Message-ID: <xmqqmtzvikwi.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <20201105133437.GC91972@coredump.intra.peff.net> (Jeff King's message of "Thu, 5 Nov 2020 08:34:37 -0500")
Jeff King <peff@peff.net> writes:
> On Thu, Nov 05, 2020 at 12:22:32AM +0000, Elijah Newren via GitGitGadget wrote:
>
>> Range-diff vs v3:
>> [...]
>> 7: 42633b8d03 ! 7: 5e8004c728 strmap: add more utility functions
>> @@ strmap.h: void *strmap_get(struct strmap *map, const char *str);
>> + * iterate through @map using @iter, @var is a pointer to a type strmap_entry
>> + */
>> +#define strmap_for_each_entry(mystrmap, iter, var) \
>> -+ for (var = hashmap_iter_first_entry_offset(&(mystrmap)->map, iter, 0); \
>> -+ var; \
>> -+ var = hashmap_iter_next_entry_offset(iter, 0))
>> ++ hashmap_for_each_entry(&(mystrmap)->map, iter, var, ent)
>> +
>> #endif /* STRMAP_H */
>> 8: ea942eb803 = 8: fd96e9fc8d strmap: enable faster clearing and reusing of strmaps
>> 9: c1d2172171 ! 9: f499934f54 strmap: add functions facilitating use as a string->int map
>> [...]
>> @@ strmap.h: static inline int strmap_empty(struct strmap *map)
>> - var; \
>> - var = hashmap_iter_next_entry_offset(iter, 0))
>> + #define strmap_for_each_entry(mystrmap, iter, var) \
>> + hashmap_for_each_entry(&(mystrmap)->map, iter, var, ent)
>>
>> +
>> +/*
>> @@ strmap.h: static inline int strmap_empty(struct strmap *map)
>
> Definitely not a problem with your patches, but I noticed this curiosity
> in the range-diff. Patch 7 changes the definition of the macro, but it
> gets mentioned again in patch 9, even though the code wasn't touched.
> The issue is that it the change from 7 ends up in the context of 9; the
> actual modification in patch 9 is in those final couple lines touching a
> comment (and they didn't change at all between the two versions).
>
> I wonder if it would be reasonable to suppress range-diff hunks in which
> all of the changed lines are context lines.
Sounds like a reasonable thing to do. As we know the shape of what
is compared in the outer diff we should be able to accurately notice
where hunk boundaries are and a hunk whose change is only on context
lines.
next prev parent reply other threads:[~2020-11-05 20:55 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-05 13:34 range-diff should suppress context-only changes? Jeff King
2020-11-05 20:55 ` Junio C Hamano [this message]
2020-11-17 21:35 ` Johannes Altmanninger
2020-11-17 21:35 ` [PATCH 1/3] range-diff: move " ## filename ##" headers to the first column Johannes Altmanninger
2020-11-17 21:35 ` [PATCH 2/3] range-diff: ignore context-only changes Johannes Altmanninger
2020-11-17 22:56 ` Eric Sunshine
2020-11-17 21:35 ` [PATCH 3/3] range-diff: only compute patch diff when patches are different Johannes Altmanninger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqmtzvikwi.fsf@gitster.c.googlers.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=newren@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.