Re: [PATCH] date: remove approxidate_relative()

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 10, 2023, 16:20 UTC
Message-ID: <xmqqjzyjemji.fsf@gitster.g>
In-Reply-To: <f5b9a290-7cec-7a83-660b-e15494d2cdc8@web.de>

René Scharfe <l.s.r@web.de> writes:

Show 7 quoted lines

> When 29f4332e66 (Quit passing 'now' to date code, 2019-09-11) removed
> its timeval parameter, approxidate_relative() became equivalent to
> approxidate().  Convert its last two call sites and remove the redundant
> function.
>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---

We might keep a backward compatibility definition or comment to help migrating in-flight (or pre-flight) callers if this were a function more often used, but seeing that the only remaining two callers are in the test helper, I agree the simplicity of straight removal like this patch does is appropriate.

Thanks, will queue.

> Formatted with -U16 for easier review.  Only useful in date.c, but
> cannot be restricted to just one file.

Perhaps

	$ git -c format-patch.date.c.context=16 format-patch -1

would be a workable idea? I do not think it makes any sense to say "when taking a diff for this path, always use 16 lines of context" so it should not be stored in a file to be used every time, like

	[format-patch "date.c"] context = 16

and it makes even less sense to say that all project participants should use this context, always, so making it an attribute would be even less appropriate.

But this was the easiest way to prototype ;-) If this turns out to be useful, the real version should probably be done by:

 * designing a command line option format, e.g.

    --per-path-context=date.c:16

 * instead of adding an extra "int ctxlen" parameter to the call
   path(s), add a ctxlen_map that maps path to ctxlen we read from
   the command line options to the diff_options structure.

 * In builtin_diff() where xecfg.ctxlen is assigned to, consult
   o->ctxlen_map and use o->context as a fallback.

or something along that line, I guess.

 diff.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git c/diff.c w/diff.c
index 469e18aed2..cbfbb4af96 100644
--- c/diff.c
+++ w/diff.c
@@ -3460,6 +3460,7 @@ static void builtin_diff(const char *name_a,
 			 const char *xfrm_msg,
 			 int must_show_header,
 			 struct diff_options *o,
+			 int ctxlen,
 			 int complete_rewrite)
 {
 	mmfile_t mf1, mf2;
@@ -3657,7 +3658,7 @@ static void builtin_diff(const char *name_a,
 		xpp.ignore_regex_nr = o->ignore_regex_nr;
 		xpp.anchors = o->anchors;
 		xpp.anchors_nr = o->anchors_nr;
-		xecfg.ctxlen = o->context;
+		xecfg.ctxlen = (ctxlen < 0) ? o->context : ctxlen;
 		xecfg.interhunkctxlen = o->interhunkcontext;
 		xecfg.flags = XDL_EMIT_FUNCNAMES;
 		if (o->flags.funccontext)
@@ -4443,6 +4444,17 @@ static void fill_metainfo(struct strbuf *msg,
 	}
 }
 
+static int configured_ctxlen(const char *path)
+{
+	struct strbuf key = STRBUF_INIT;
+	int ctxlen;
+
+	strbuf_addf(&key, "format-patch.%s.context", path);
+	if (!repo_config_get_int(the_repository, key.buf, &ctxlen))
+		return ctxlen;
+	return -1;
+}
+
 static void run_diff_cmd(const char *pgm,
 			 const char *name,
 			 const char *other,
@@ -4480,12 +4492,14 @@ static void run_diff_cmd(const char *pgm,
 		return;
 	}
 	if (one && two) {
+		int ctxlen = configured_ctxlen(attr_path);
+
 		if (!o->ignore_driver_algorithm && drv && drv->algorithm)
 			set_diff_algorithm(o, drv->algorithm);
 
 		builtin_diff(name, other ? other : name,
 			     one, two, xfrm_msg, must_show_header,
-			     o, complete_rewrite);
+			     o, ctxlen, complete_rewrite);
 	} else {
 		fprintf(o->file, "* Unmerged path %s\n", name);
 	}