Re: [PATCH 1/6] config: add helper function for parsing key names
On Mon, Jan 14, 2013 at 10:08:47AM -0800, Junio C Hamano wrote: +extern int match_config_key(const char *var, +const char *section, +const char **subsection, int *subsection_len, +const char **key); + I agree with Jonathan about the naming s/match/parse/. I see this is marked for re-roll in WC. I'm happy to re-roll it with the suggestions from Jonathan, but before I do, did you have any comment on the struct config_key alternative I sent as a follow-up? You said: After looking at the callers in your later patches, I think the counted interface to subsection is probably fine. The caller can check !subsection to see if it is a two- or three- level name, and if (parse_config_key(var, submodule, name, namelen, key) 0 || !name) return 0; is very easy to follow (that is the result of your 5th step). but I wasn't sure if that was it is not worth the trouble of the other one or I did not yet read the other one. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] config: add helper function for parsing key names
Jeff King p...@peff.net writes: ... did you have any comment on the struct config_key alternative I sent as a follow-up? I did read it but I cannot say I did so very carefully. My gut reaction was that the take the variable name and section name, return the subsection name pointer and length, if there is any, and the key made it readable enough. The proposed interface to make and lend a copy to the caller does make it more readble, but I do not know if that is worth doing. Neutral-to-slightly-in-favor, I would say. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] config: add helper function for parsing key names
The config callback functions get keys of the general form:
section.subsection.key
(where the subsection may be contain arbitrary data, or may
be missing). For matching keys without subsections, it is
simple enough to call strcmp. Matching keys with
subsections is a little more complicated, and each callback
does it in an ad-hoc way, usually involving error-prone
pointer arithmetic.
Let's provide a helper that keeps the pointer arithmetic all
in one place.
Signed-off-by: Jeff King p...@peff.net
---
No users yet; they come in future patches.
cache.h | 15 +++
config.c | 33 +
2 files changed, 48 insertions(+)
diff --git a/cache.h b/cache.h
index c257953..14003b8 100644
--- a/cache.h
+++ b/cache.h
@@ -1164,6 +1164,21 @@ extern int git_config_include(const char *name, const
char *value, void *data);
#define CONFIG_INCLUDE_INIT { 0 }
extern int git_config_include(const char *name, const char *value, void *data);
+/*
+ * Match and parse a config key of the form:
+ *
+ * section.(subsection.)?key
+ *
+ * (i.e., what gets handed to a config_fn_t). The caller provides the section;
+ * we return -1 if it does not match, 0 otherwise. The subsection and key
+ * out-parameters are filled by the function (and subsection is NULL if it is
+ * missing).
+ */
+extern int match_config_key(const char *var,
+const char *section,
+const char **subsection, int *subsection_len,
+const char **key);
+
extern int committer_ident_sufficiently_given(void);
extern int author_ident_sufficiently_given(void);
diff --git a/config.c b/config.c
index 7b444b6..18d9c0e 100644
--- a/config.c
+++ b/config.c
@@ -1667,3 +1667,36 @@ int config_error_nonbool(const char *var)
{
return error(Missing value for '%s', var);
}
+
+int match_config_key(const char *var,
+const char *section,
+const char **subsection, int *subsection_len,
+const char **key)
+{
+ int section_len = strlen(section);
+ const char *dot;
+
+ /* Does it start with section. ? */
+ if (prefixcmp(var, section) || var[section_len] != '.')
+ return -1;
+
+ /*
+* Find the key; we don't know yet if we have a subsection, but we must
+* parse backwards from the end, since the subsection may have dots in
+* it, too.
+*/
+ dot = strrchr(var, '.');
+ *key = dot + 1;
+
+ /* Did we have a subsection at all? */
+ if (dot == var + section_len) {
+ *subsection = NULL;
+ *subsection_len = 0;
+ }
+ else {
+ *subsection = var + section_len + 1;
+ *subsection_len = dot - *subsection;
+ }
+
+ return 0;
+}
--
1.8.1.rc1.10.g7d71f7b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] config: add helper function for parsing key names
Jeff King p...@peff.net writes:
The config callback functions get keys of the general form:
section.subsection.key
(where the subsection may be contain arbitrary data, or may
be missing). For matching keys without subsections, it is
simple enough to call strcmp. Matching keys with
subsections is a little more complicated, and each callback
does it in an ad-hoc way, usually involving error-prone
pointer arithmetic.
Let's provide a helper that keeps the pointer arithmetic all
in one place.
Signed-off-by: Jeff King p...@peff.net
---
No users yet; they come in future patches.
cache.h | 15 +++
config.c | 33 +
2 files changed, 48 insertions(+)
diff --git a/cache.h b/cache.h
index c257953..14003b8 100644
--- a/cache.h
+++ b/cache.h
@@ -1164,6 +1164,21 @@ extern int git_config_include(const char *name, const
char *value, void *data);
#define CONFIG_INCLUDE_INIT { 0 }
extern int git_config_include(const char *name, const char *value, void
*data);
+/*
+ * Match and parse a config key of the form:
+ *
+ * section.(subsection.)?key
+ *
+ * (i.e., what gets handed to a config_fn_t). The caller provides the
section;
+ * we return -1 if it does not match, 0 otherwise. The subsection and key
+ * out-parameters are filled by the function (and subsection is NULL if it is
+ * missing).
+ */
+extern int match_config_key(const char *var,
+ const char *section,
+ const char **subsection, int *subsection_len,
+ const char **key);
+
I agree with Jonathan about the naming s/match/parse/.
After looking at the callers in your later patches, I think the
counted interface to subsection is probably fine. The caller can
check !subsection to see if it is a two- or three- level name, and
if (parse_config_key(var, submodule, name, namelen, key) 0 ||
!name)
return 0;
is very easy to follow (that is the result of your 5th step).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
