Stefan Beller [off-list ref] writes:
Yes; though I'd place it in strbuf.{c,h} as it is operating
on the internals of the strbuf. (Do we make any promises outside of
strbuf about the internals? I mean we use .buf all the time, so maybe
I am overly cautious here)
I'd rather have it not use struct strbuf as an interface. It only
needs to pass "char *" and its promise that it touches the string
in-place without changing the length need to be documented as a
comment before the function.
quoted
config.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/config.c b/config.c
index c6b874a7bf..98bf8fee32 100644
--- a/config.c
+++ b/config.c
@@ -201,6 +201,20 @@ void git_config_push_parameter(const char *text)
strbuf_release(&env);
}
+static void canonicalize_config_variable_name(struct strbuf *var)
+{
+ char *first_dot = strchr(var->buf, '.');
+ char *last_dot = strrchr(var->buf, '.');
If first_dot != NULL, then last_dot !+ NULL as well.
(either both are NULL or none of them),
so we can loose one condition below.
I do not think it is worth it, though.
quoted
+ char *cp;
+
+ if (first_dot)
+ for (cp = var->buf; *cp && cp < first_dot; cp++)
+ *cp = tolower(*cp);
+ if (last_dot)
+ for (cp = last_dot; *cp; cp++)
+ *cp = tolower(*cp);
if (first_dot) {
scan up to first dot
if (last_dot)
scan from last dot to the end
}
would be uglier.
On Wed, Feb 15, 2017 at 3:37 PM, Junio C Hamano [off-list ref] wrote:
Stefan Beller [off-list ref] writes:
quoted
Yes; though I'd place it in strbuf.{c,h} as it is operating
on the internals of the strbuf. (Do we make any promises outside of
strbuf about the internals? I mean we use .buf all the time, so maybe
I am overly cautious here)
I'd rather have it not use struct strbuf as an interface. It only
needs to pass "char *" and its promise that it touches the string
in-place without changing the length need to be documented as a
comment before the function.
quoted
quoted
config.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/config.c b/config.c
index c6b874a7bf..98bf8fee32 100644
--- a/config.c
+++ b/config.c
@@ -201,6 +201,20 @@ void git_config_push_parameter(const char *text)
strbuf_release(&env);
}
+static void canonicalize_config_variable_name(struct strbuf *var)
+{
+ char *first_dot = strchr(var->buf, '.');
+ char *last_dot = strrchr(var->buf, '.');
If first_dot != NULL, then last_dot !+ NULL as well.
(either both are NULL or none of them),
so we can loose one condition below.
I do not think it is worth it, though.
quoted
quoted
+ char *cp;
+
+ if (first_dot)
+ for (cp = var->buf; *cp && cp < first_dot; cp++)
+ *cp = tolower(*cp);
+ if (last_dot)
+ for (cp = last_dot; *cp; cp++)
+ *cp = tolower(*cp);
if (first_dot) {
scan up to first dot
if (last_dot)
just leave out the 'if (last_dot)' ?
if (first_dot) {
/* also implies last_dot */
do 0 -> first
do last -> end
}
Ahh, that would work, too.
On Wed, Feb 15, 2017 at 3:43 PM, Stefan Beller [off-list ref] wrote:
On Wed, Feb 15, 2017 at 3:37 PM, Junio C Hamano [off-list ref] wrote:
quoted
Stefan Beller [off-list ref] writes:
quoted
Yes; though I'd place it in strbuf.{c,h} as it is operating
on the internals of the strbuf. (Do we make any promises outside of
strbuf about the internals? I mean we use .buf all the time, so maybe
I am overly cautious here)
I'd rather have it not use struct strbuf as an interface. It only
needs to pass "char *" and its promise that it touches the string
in-place without changing the length need to be documented as a
comment before the function.
quoted
quoted
config.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/config.c b/config.c
index c6b874a7bf..98bf8fee32 100644
--- a/config.c
+++ b/config.c
@@ -201,6 +201,20 @@ void git_config_push_parameter(const char *text)
strbuf_release(&env);
}
+static void canonicalize_config_variable_name(struct strbuf *var)
+{
+ char *first_dot = strchr(var->buf, '.');
+ char *last_dot = strrchr(var->buf, '.');
If first_dot != NULL, then last_dot !+ NULL as well.
(either both are NULL or none of them),
so we can loose one condition below.
I do not think it is worth it, though.
quoted
quoted
+ char *cp;
+
+ if (first_dot)
+ for (cp = var->buf; *cp && cp < first_dot; cp++)
+ *cp = tolower(*cp);
+ if (last_dot)
+ for (cp = last_dot; *cp; cp++)
+ *cp = tolower(*cp);
if (first_dot) {
scan up to first dot
if (last_dot)
just leave out the 'if (last_dot)' ?
if (first_dot) {
/* also implies last_dot */
do 0 -> first
do last -> end
}
On Wed, Feb 15, 2017 at 03:37:37PM -0800, Junio C Hamano wrote:
Stefan Beller [off-list ref] writes:
quoted
Yes; though I'd place it in strbuf.{c,h} as it is operating
on the internals of the strbuf. (Do we make any promises outside of
strbuf about the internals? I mean we use .buf all the time, so maybe
I am overly cautious here)
I'd rather have it not use struct strbuf as an interface. It only
needs to pass "char *" and its promise that it touches the string
in-place without changing the length need to be documented as a
comment before the function.
This code also uses the hacky strbuf_split() interface. It would be nice
to one day move off of it (the only other strbuf-specific function used
there is strbuf_trim).
One _could_ actually parse the whole thing left-to-right (soaking up
whitespace and doing the canonicalizing) instead of dealing with a split
function at all. But the canonicalize bit you added here would not be
reusable then. And it's probably not worth holding up the bugfix here.
quoted
quoted
+static void canonicalize_config_variable_name(struct strbuf *var)
+{
+ char *first_dot = strchr(var->buf, '.');
+ char *last_dot = strrchr(var->buf, '.');
If first_dot != NULL, then last_dot !+ NULL as well.
(either both are NULL or none of them),
so we can loose one condition below.
I do not think it is worth it, though.
If you really want to be picky, you do not need to find the first dot
at all. You can downcase everything until you see a dot, and then
find the last dot (if any) from there.
I don't think it matters much in practice.
-Peff