Re: [PATCH] date: remove approxidate_relative()

From: Jeff King <peff@peff.net>
Date: Apr 11, 2023, 09:30 UTC
Message-ID: <20230411093005.GB398350@coredump.intra.peff.net>
In-Reply-To: <xmqqy1mzcus6.fsf@gitster.g>

On Mon, Apr 10, 2023 at 02:05:45PM -0700, Junio C Hamano wrote:

Show 8 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > Cute. It feels like this only goes half-way, though. You really want
> > per-hunk configurable context. This particular patch was just lucky that
> > there was only one hunk in the date.c file.
> 
> "-U16" extends the context lines in both directions by the same
> number of lines, but most likely you need to extend asymmetrically.

Good point. This is sort of like grep's "-C" along with "-A" and "-B". All of which I generally find somewhat awkward. :)

Show 17 quoted lines

> In René's patch, for example, much of the body of approxidate_str()
> is visible in the precontext of the hunk to remove
> approxidate_relative(), but the body of that function is irrelevant.
> What he wanted to show was the body of approxidate_careful() and the
> size of that function is where the -U16 came from.
> 
> Instead, imagine --extra-context='<range>:<path>' were the way to
> tell Git to include the specified range of lines in the post context
> even though they may not have been modified.  Then René's patch
> could have been produced with
> 
>     $ git format-patch -1 \
>       --extra-context='/^timestamp_t approxidate_careful/,/^}$/:date.c'
> 
> and would have shown 3 lines of precontext before the removed
> approxidate_relative(), plus the unchanged approxidate_careful()
> function in full in the postcontext.

Ooh, I like that very much. In that sense it really feels like an extension of --function-context. Would the regexes be searches starting from the edge of some context (as they more or less are under the hood for function context), or would you search within the whole file for ranges (and then presumably use them when a hunk's context is adjacent to or overlaps a range)?

If the latter, I guess you could also allow both absolute and relative line numbers, similar to how "-L" accepts range input.

-Peff