Jeff King [off-list ref] writes:
On Fri, Jan 04, 2008 at 03:59:34AM -0500, Jeff King wrote:
quoted
OK. In that case, we need a way for the plumbing to tell the diff
machinery "don't ever try loading the ui config."
quoted
quoted
* funcname-pattern can go either way; that affects what appears
at the end of @@ context @@ lines, and would not have risk to
corrupt the patch for plumbing.
I just sent a patch to put the funcname pattern in the "basic" config,
and to get rid of the lazy config loading. So that fixes one call by the
plumbing to read_config_if_needed.
And it looks like the second call is already OK. We don't try parsing
the config to get the external diff command unless ALLOW_EXTERNAL is
set, which the plumbing already disallows (though I am still confused
why it would need to be loaded lazily in the first place -- I wonder if
read_config_if_needed is needed at all).
I think that was a premature optimization without benching. It
is expected that most trees would not have attributes to define
custom low-level diff types, and without them we do not need to
parse the configuration to find out the external commands to be
used.
On Fri, Jan 04, 2008 at 01:37:46AM -0800, Junio C Hamano wrote:
quoted
And it looks like the second call is already OK. We don't try parsing
the config to get the external diff command unless ALLOW_EXTERNAL is
set, which the plumbing already disallows (though I am still confused
why it would need to be loaded lazily in the first place -- I wonder if
read_config_if_needed is needed at all).
I think that was a premature optimization without benching. It
is expected that most trees would not have attributes to define
custom low-level diff types, and without them we do not need to
parse the configuration to find out the external commands to be
used.
Ah. But we were parsing them anyway in git_diff_ui_config at the
beginning of the program (and we need to, since you will always have
diff.default.*). So I think this is safe to do, and can replace my other
"only read config once" patch from a few minutes ago:
-- >8 --
diff: remove lazy config loading
There is no point to this. Either:
1. The program has already loaded git_diff_ui_config, in
which case this is a noop.
2. The program didn't, which means it is plumbing that
does not _want_ git_diff_ui_config to be loaded.
Signed-off-by: Jeff King <redacted>
---
diff.c | 12 ------------
1 files changed, 0 insertions(+), 12 deletions(-)
diff --git a/diff.c b/diff.c
index ecf2fd6..2c78d74 100644
--- a/diff.c
+++ b/diff.c
@@ -59,17 +59,6 @@ static struct ll_diff_driver {
char *cmd;
} *user_diff, **user_diff_tail;
-static void read_config_if_needed(void)
-{
- static int done = 0;
- if (done)
- return;
-
- user_diff_tail = &user_diff;
- git_config(git_diff_ui_config);
- done = 1;
-}
-
/*
* Currently there is only "diff.<drivername>.command" variable;
* because there are "diff.color.<slot>" variables, we are parsing@@ -1825,7 +1814,6 @@ static const char *external_diff_attr(const char *name)
!ATTR_UNSET(value)) {
struct ll_diff_driver *drv;
- read_config_if_needed();
for (drv = user_diff; drv; drv = drv->next)
if (!strcmp(drv->name, value))
return drv->cmd;--
1.5.4.rc2.1125.ga305e-dirty