git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: Git List <git@vger.kernel.org>
Subject: Re: [PATCH] date: remove approxidate_relative()
Date: Mon, 10 Apr 2023 09:20:49 -0700	[thread overview]
Message-ID: <xmqqjzyjemji.fsf@gitster.g> (raw)
In-Reply-To: <f5b9a290-7cec-7a83-660b-e15494d2cdc8@web.de> ("René Scharfe"'s message of "Sat, 8 Apr 2023 11:35:24 +0200")

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

> 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);
 	}


  reply	other threads:[~2023-04-10 16:20 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-08  9:35 [PATCH] date: remove approxidate_relative() René Scharfe
2023-04-10 16:20 ` Junio C Hamano [this message]
2023-04-10 20:25   ` Jeff King
2023-04-10 20:52     ` René Scharfe
2023-04-11  9:25       ` Jeff King
2023-04-10 21:05     ` Junio C Hamano
2023-04-11  9:30       ` Jeff King
2023-04-11 15:43         ` Junio C Hamano
2023-04-12  7:30           ` Jeff King

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=xmqqjzyjemji.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).