* [PATCH 0/2] Section renaming can lose content
@ 2009-07-24 21:21 Alex Vandiver
2009-07-24 21:21 ` [PATCH 1/2] Make section_name_match start on '[', and return the length on success Alex Vandiver
0 siblings, 1 reply; 17+ messages in thread
From: Alex Vandiver @ 2009-07-24 21:21 UTC (permalink / raw)
To: git
I submitted a patch to the testfile on the 8th; what follows is a pass
at solving the underlying bug as well.
- Alex
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/2] Make section_name_match start on '[', and return the length on success
2009-07-24 21:21 [PATCH 0/2] Section renaming can lose content Alex Vandiver
@ 2009-07-24 21:21 ` Alex Vandiver
2009-07-24 21:21 ` [PATCH 2/2] After renaming a section, print any trailing variable definitions Alex Vandiver
2009-07-25 14:09 ` [PATCH 1/2] Make section_name_match start on '[', and return the length on success Johannes Schindelin
0 siblings, 2 replies; 17+ messages in thread
From: Alex Vandiver @ 2009-07-24 21:21 UTC (permalink / raw)
To: git
Signed-off-by: Alex Vandiver <alex@chmrr.net>
---
config.c | 19 ++++++++++++++++---
1 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/config.c b/config.c
index 04380bb..7d6f6f6 100644
--- a/config.c
+++ b/config.c
@@ -1194,7 +1194,9 @@ write_err_out:
static int section_name_match (const char *buf, const char *name)
{
int i = 0, j = 0, dot = 0;
- for (; buf[i] && buf[i] != ']'; i++) {
+ if (buf[i] != '[')
+ return 0;
+ for (i = 1; buf[i] && buf[i] != ']'; i++) {
if (!dot && isspace(buf[i])) {
dot = 1;
if (name[j++] != '.')
@@ -1215,7 +1217,17 @@ static int section_name_match (const char *buf, const char *name)
if (buf[i] != name[j++])
break;
}
- return (buf[i] == ']' && name[j] == 0);
+ if (buf[i] == ']' && name[j] == 0) {
+ /*
+ * We match, now just find the right length offset by
+ * gobbling up any whitespace after it, as well
+ */
+ i++;
+ for (; buf[i] && isspace(buf[i]); i++)
+ ; /* do nothing */
+ return i;
+ }
+ return 0;
}
/* if new_name == NULL, the section is removed instead */
@@ -1249,7 +1261,8 @@ int git_config_rename_section(const char *old_name, const char *new_name)
; /* do nothing */
if (buf[i] == '[') {
/* it's a section */
- if (section_name_match (&buf[i+1], old_name)) {
+ int offset = section_name_match (&buf[i], old_name);
+ if (offset > 0) {
ret++;
if (new_name == NULL) {
remove = 1;
--
1.6.3.3.473.gb74fc4.dirty
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] After renaming a section, print any trailing variable definitions
2009-07-24 21:21 ` [PATCH 1/2] Make section_name_match start on '[', and return the length on success Alex Vandiver
@ 2009-07-24 21:21 ` Alex Vandiver
2009-07-24 22:11 ` Nanako Shiraishi
2009-07-25 14:09 ` [PATCH 1/2] Make section_name_match start on '[', and return the length on success Johannes Schindelin
1 sibling, 1 reply; 17+ messages in thread
From: Alex Vandiver @ 2009-07-24 21:21 UTC (permalink / raw)
To: git
Signed-off-by: Alex Vandiver <alex@chmrr.net>
---
config.c | 22 +++++++++++++++++++---
t/t1300-repo-config.sh | 22 ++++++++++++++++++++++
2 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/config.c b/config.c
index 7d6f6f6..3a2965e 100644
--- a/config.c
+++ b/config.c
@@ -1257,6 +1257,7 @@ int git_config_rename_section(const char *old_name, const char *new_name)
while (fgets(buf, sizeof(buf), config_file)) {
int i;
int length;
+ char* output = buf;
for (i = 0; buf[i] && isspace(buf[i]); i++)
; /* do nothing */
if (buf[i] == '[') {
@@ -1273,14 +1274,29 @@ int git_config_rename_section(const char *old_name, const char *new_name)
ret = write_error(lock->filename);
goto out;
}
- continue;
+ /*
+ * We wrote out the new section, with
+ * a newline, now skip the old
+ * section's length
+ */
+ output += offset + i;
+ if (strlen(output) > 0) {
+ /*
+ * More content means there's
+ * a declaration to put on the
+ * next line; indent with a
+ * tab
+ */
+ output -= 1;
+ output[0] = '\t';
+ }
}
remove = 0;
}
if (remove)
continue;
- length = strlen(buf);
- if (write_in_full(out_fd, buf, length) != length) {
+ length = strlen(output);
+ if (write_in_full(out_fd, output, length) != length) {
ret = write_error(lock->filename);
goto out;
}
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 43ea283..8c43dcd 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -460,6 +460,28 @@ EOF
test_expect_success "rename succeeded" "test_cmp expect .git/config"
cat >> .git/config << EOF
+[branch "vier"] z = 1
+EOF
+
+test_expect_success "rename a section with a var on the same line" \
+ 'git config --rename-section branch.vier branch.zwei'
+
+cat > expect << EOF
+# Hallo
+ #Bello
+[branch "zwei"]
+ x = 1
+[branch "zwei"]
+ y = 1
+[branch "drei"]
+weird
+[branch "zwei"]
+ z = 1
+EOF
+
+test_expect_success "rename succeeded" "test_cmp expect .git/config"
+
+cat >> .git/config << EOF
[branch "zwei"] a = 1 [branch "vier"]
EOF
--
1.6.3.3.473.gb74fc4.dirty
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] After renaming a section, print any trailing variable definitions
2009-07-24 21:21 ` [PATCH 2/2] After renaming a section, print any trailing variable definitions Alex Vandiver
@ 2009-07-24 22:11 ` Nanako Shiraishi
2009-07-24 22:26 ` Alex Vandiver
2009-07-24 23:39 ` [PATCH 2/2] After renaming a section, print any trailing variable definitions Junio C Hamano
0 siblings, 2 replies; 17+ messages in thread
From: Nanako Shiraishi @ 2009-07-24 22:11 UTC (permalink / raw)
To: Alex Vandiver; +Cc: git
Quoting Alex Vandiver <alex@chmrr.net>
> diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
> index 43ea283..8c43dcd 100755
> --- a/t/t1300-repo-config.sh
> +++ b/t/t1300-repo-config.sh
> @@ -460,6 +460,28 @@ EOF
> test_expect_success "rename succeeded" "test_cmp expect .git/config"
>
> cat >> .git/config << EOF
> +[branch "vier"] z = 1
> +EOF
Isn't this a syntax error?
Documentation/config.txt says this.
Subsection names are case sensitive and can contain any characters
except newline (doublequote `"` and backslash have to be escaped as
`\"` and `\\`, respectively). Section headers cannot span multiple
lines. Variables may belong directly to a section or to a given
subsection. You can have `[section]` if you have `[section
"subsection"]`, but you don't need to.
There is also a case insensitive alternative `[section.subsection]`
syntax. In this syntax, subsection names follow the same restrictions
as for section names.
All the other lines are recognized as setting variables, in the form
'name = value'. If there is no equal sign on the line, the entire
line ...
I read "All the other lines" to mean that the section headers and variable definitions are supposed to be on different lines.
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] After renaming a section, print any trailing variable definitions
2009-07-24 22:11 ` Nanako Shiraishi
@ 2009-07-24 22:26 ` Alex Vandiver
2009-07-26 16:18 ` [PATCH] Make git config fail on variables with no section, as documented Alex Vandiver
2009-07-24 23:39 ` [PATCH 2/2] After renaming a section, print any trailing variable definitions Junio C Hamano
1 sibling, 1 reply; 17+ messages in thread
From: Alex Vandiver @ 2009-07-24 22:26 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: git
At Fri Jul 24 18:11:05 -0400 2009, Nanako Shiraishi wrote:
> Quoting Alex Vandiver <alex@chmrr.net>
> > diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
> > index 43ea283..8c43dcd 100755
> > --- a/t/t1300-repo-config.sh
> > +++ b/t/t1300-repo-config.sh
> > @@ -460,6 +460,28 @@ EOF
> > test_expect_success "rename succeeded" "test_cmp expect .git/config"
> >
> > cat >> .git/config << EOF
> > +[branch "vier"] z = 1
> > +EOF
>
> Isn't this a syntax error?
Nope -- at least, not according to both the code, and the tests
(search or noNewline in t/t1300-repo-config.sh).
Though I also note that the documentation disagrees with the code in
the following case:
Each variable must belong to some section, which means that there
must be a section header before the first setting of a variable.
$ cat >bogus
foo = 42
$ git config --file bogus --list
foo=42
$ git config --file bogus --get foo
42
- Alex
--
Networking -- only one letter away from not working
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] Make git config fail on variables with no section, as documented
2009-07-24 22:26 ` Alex Vandiver
@ 2009-07-26 16:18 ` Alex Vandiver
2009-07-26 16:49 ` Johannes Schindelin
0 siblings, 1 reply; 17+ messages in thread
From: Alex Vandiver @ 2009-07-26 16:18 UTC (permalink / raw)
To: git
Documentation/config.txt claims:
Each variable must belong to some section, which means that there
must be a section header before the first setting of a variable.
However, the parsing code did not enforce this. This change makes it
a syntax error to defined a variable before the first section header.
Signed-off-by: Alex Vandiver <alex@chmrr.net>
---
config.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/config.c b/config.c
index 1b3823d..447ad00 100644
--- a/config.c
+++ b/config.c
@@ -248,6 +248,8 @@ static int git_parse_file(config_fn_t fn, void *data)
}
if (!isalpha(c))
break;
+ if (baselen == 0)
+ break;
var[baselen] = tolower(c);
if (get_value(fn, data, var, baselen+1) < 0)
break;
--
1.6.3.3.473.gb74fc4.dirty
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] Make git config fail on variables with no section, as documented
2009-07-26 16:18 ` [PATCH] Make git config fail on variables with no section, as documented Alex Vandiver
@ 2009-07-26 16:49 ` Johannes Schindelin
2009-07-26 19:32 ` Alex Vandiver
0 siblings, 1 reply; 17+ messages in thread
From: Johannes Schindelin @ 2009-07-26 16:49 UTC (permalink / raw)
To: Alex Vandiver; +Cc: git
Hi,
On Sun, 26 Jul 2009, Alex Vandiver wrote:
> Documentation/config.txt claims:
>
> Each variable must belong to some section, which means that there
> must be a section header before the first setting of a variable.
>
> However, the parsing code did not enforce this. This change makes it
> a syntax error to defined a variable before the first section header.
Is there any downside in allowing this?
Ciao,
Dscho
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Make git config fail on variables with no section, as documented
2009-07-26 16:49 ` Johannes Schindelin
@ 2009-07-26 19:32 ` Alex Vandiver
2009-07-26 19:48 ` Johannes Schindelin
0 siblings, 1 reply; 17+ messages in thread
From: Alex Vandiver @ 2009-07-26 19:32 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
At Sun Jul 26 12:49:28 -0400 2009, Johannes Schindelin wrote:
> Is there any downside in allowing this?
Not explicitly. However, there are no legal current uses of it, and
allowing it might encourage extensions to use the top-level config
namespace. It also has the odd property that it _must_ be at the top
of a configuration file -- unlike all other configuration options, you
can never return to the section to add more variable definitions
later.
- Alex
--
Networking -- only one letter away from not working
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Make git config fail on variables with no section, as documented
2009-07-26 19:32 ` Alex Vandiver
@ 2009-07-26 19:48 ` Johannes Schindelin
2009-07-26 20:24 ` Alex Vandiver
0 siblings, 1 reply; 17+ messages in thread
From: Johannes Schindelin @ 2009-07-26 19:48 UTC (permalink / raw)
To: Alex Vandiver; +Cc: git
Hi,
On Sun, 26 Jul 2009, Alex Vandiver wrote:
> At Sun Jul 26 12:49:28 -0400 2009, Johannes Schindelin wrote:
> > Is there any downside in allowing this?
>
> Not explicitly. However, there are no legal current uses of it, and
> allowing it might encourage extensions to use the top-level config
> namespace.
So?
> It also has the odd property that it _must_ be at the top of a
> configuration file -- unlike all other configuration options, you can
> never return to the section to add more variable definitions later.
Yes, that is a special property that you might actually want in some
contexts.
That, together with the fact that "git config -f <file>" was meant
_explicitely_ to allow 3rd party porcelains having their own config files
without having to implement their own "git config" lets me suspect that
we'd rather want the current behavior.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Make git config fail on variables with no section, as documented
2009-07-26 19:48 ` Johannes Schindelin
@ 2009-07-26 20:24 ` Alex Vandiver
0 siblings, 0 replies; 17+ messages in thread
From: Alex Vandiver @ 2009-07-26 20:24 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
At Sun Jul 26 15:48:20 -0400 2009, Johannes Schindelin wrote:
> That, together with the fact that "git config -f <file>" was meant
> _explicitely_ to allow 3rd party porcelains having their own config files
> without having to implement their own "git config" lets me suspect that
> we'd rather want the current behavior.
*shrug* Note I'm not wholly convinced of the rationale myself -- I'm
merely trying to reconcile the documentation and reality. I'll also
note that you can't, at current, set or unset such values from the
command-line.
Looking back at the blame for the documentation, it looks like e136f33
in 2007 is what added the claim that section-less variables weren't
acceptable. The `git repo-config` of the time parsed them just fine,
however.
If you wish to correct the documentation instead, I can send in a doc
patch. Fixing --set and --unset to work with section-less variables
will take a bit more work, however.
- Alex
--
Networking -- only one letter away from not working
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] After renaming a section, print any trailing variable definitions
2009-07-24 22:11 ` Nanako Shiraishi
2009-07-24 22:26 ` Alex Vandiver
@ 2009-07-24 23:39 ` Junio C Hamano
2009-07-25 0:28 ` (unknown), Nanako Shiraishi
2009-07-25 14:10 ` [PATCH 2/2] After renaming a section, print any trailing variable definitions Johannes Schindelin
1 sibling, 2 replies; 17+ messages in thread
From: Junio C Hamano @ 2009-07-24 23:39 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: Alex Vandiver, git, Johannes Schindelin
Nanako Shiraishi <nanako3@lavabit.com> writes:
> Quoting Alex Vandiver <alex@chmrr.net>
>
>> diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
>> index 43ea283..8c43dcd 100755
>> --- a/t/t1300-repo-config.sh
>> +++ b/t/t1300-repo-config.sh
>> @@ -460,6 +460,28 @@ EOF
>> test_expect_success "rename succeeded" "test_cmp expect .git/config"
>>
>> cat >> .git/config << EOF
>> +[branch "vier"] z = 1
>> +EOF
>
> Isn't this a syntax error?
>
> Documentation/config.txt says this.
Even if it were, I think it would be nice to allow it. I'll have to
re-read Alex's patch, but I thought it was sane. Perhaps we can update
the documentation, mildly hinting that it is allowed without encouraging
it too strongly, as I think it is a good style to have these on separate
lines.
Dscho? Have any suggestions/comments on the patch?
^ permalink raw reply [flat|nested] 17+ messages in thread
* (unknown),
2009-07-24 23:39 ` [PATCH 2/2] After renaming a section, print any trailing variable definitions Junio C Hamano
@ 2009-07-25 0:28 ` Nanako Shiraishi
2009-07-25 14:10 ` [PATCH 2/2] After renaming a section, print any trailing variable definitions Johannes Schindelin
1 sibling, 0 replies; 17+ messages in thread
From: Nanako Shiraishi @ 2009-07-25 0:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Alex Vandiver, git, Johannes Schindelin
Subject: [PATCH] Documentation/config.txt: a variable can be defined on the section header line
Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com>
---
Quoting Junio C Hamano <gitster@pobox.com>:
> Nanako Shiraishi <nanako3@lavabit.com> writes:
>
>> Isn't this a syntax error?
>>
>> Documentation/config.txt says this.
>
> Even if it were, I think it would be nice to allow it. I'll have to
> re-read Alex's patch, but I thought it was sane. Perhaps we can update
> the documentation, mildly hinting that it is allowed without encouraging
> it too strongly, as I think it is a good style to have these on separate
> lines.
How about this small update to the documentation, then?
Documentation/config.txt | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6857d2f..c6f09f8 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -49,7 +49,8 @@ There is also a case insensitive alternative `[section.subsection]` syntax.
In this syntax, subsection names follow the same restrictions as for section
names.
-All the other lines are recognized as setting variables, in the form
+All the other lines (and the remainder of the line after the section
+header) are recognized as setting variables, in the form
'name = value'. If there is no equal sign on the line, the entire line
is taken as 'name' and the variable is recognized as boolean "true".
The variable names are case-insensitive and only alphanumeric
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] After renaming a section, print any trailing variable definitions
2009-07-24 23:39 ` [PATCH 2/2] After renaming a section, print any trailing variable definitions Junio C Hamano
2009-07-25 0:28 ` (unknown), Nanako Shiraishi
@ 2009-07-25 14:10 ` Johannes Schindelin
1 sibling, 0 replies; 17+ messages in thread
From: Johannes Schindelin @ 2009-07-25 14:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, Alex Vandiver, git
Hi,
On Fri, 24 Jul 2009, Junio C Hamano wrote:
> Dscho? Have any suggestions/comments on the patch?
Thanks for making me aware of the patches. I just sent out a reply.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] Make section_name_match start on '[', and return the length on success
2009-07-24 21:21 ` [PATCH 1/2] Make section_name_match start on '[', and return the length on success Alex Vandiver
2009-07-24 21:21 ` [PATCH 2/2] After renaming a section, print any trailing variable definitions Alex Vandiver
@ 2009-07-25 14:09 ` Johannes Schindelin
2009-07-25 17:18 ` Alex Vandiver
1 sibling, 1 reply; 17+ messages in thread
From: Johannes Schindelin @ 2009-07-25 14:09 UTC (permalink / raw)
To: Alex Vandiver; +Cc: git
Hi,
On Fri, 24 Jul 2009, Alex Vandiver wrote:
> diff --git a/config.c b/config.c
> index 04380bb..7d6f6f6 100644
> --- a/config.c
> +++ b/config.c
> @@ -1194,7 +1194,9 @@ write_err_out:
> static int section_name_match (const char *buf, const char *name)
> {
> int i = 0, j = 0, dot = 0;
> - for (; buf[i] && buf[i] != ']'; i++) {
> + if (buf[i] != '[')
> + return 0;
Is this not unnecessary, given that we only call that function when we
know that buf[0] == '[':
> @@ -1249,7 +1261,8 @@ int git_config_rename_section(const char *old_name, const char *new_name)
> ; /* do nothing */
> if (buf[i] == '[') {
> /* it's a section */
> - if (section_name_match (&buf[i+1], old_name)) {
> + int offset = section_name_match (&buf[i], old_name);
> + if (offset > 0) {
> ret++;
> if (new_name == NULL) {
> remove = 1;
I was a bit surprised that "offset" is not used further in your patch, but
I saw that 2/2 uses it.
So except for the unnecessary test, I like your patches (read: ACK).
Ciao,
Dscho
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] Make section_name_match start on '[', and return the length on success
2009-07-25 14:09 ` [PATCH 1/2] Make section_name_match start on '[', and return the length on success Johannes Schindelin
@ 2009-07-25 17:18 ` Alex Vandiver
2009-07-25 17:39 ` Junio C Hamano
2009-07-25 17:41 ` Johannes Schindelin
0 siblings, 2 replies; 17+ messages in thread
From: Alex Vandiver @ 2009-07-25 17:18 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
At Sat Jul 25 10:09:56 -0400 2009, Johannes Schindelin wrote:
> Is this not unnecessary, given that we only call that function when we
> know that buf[0] == '[':
Yes. However, given that I had changed the calling convention for the
code, (it used to be passed the string starting just _inside_ the
'['), I wanted to make the new calling convention clearer, and catch
any places that were using the old convention.
I'm happy to submit a new version without it, if you wish.
> I was a bit surprised that "offset" is not used further in your patch, but
> I saw that 2/2 uses it.
Yeah, this hunk should probably have gone in 2/2 instead.
- Alex
--
Networking -- only one letter away from not working
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] Make section_name_match start on '[', and return the length on success
2009-07-25 17:18 ` Alex Vandiver
@ 2009-07-25 17:39 ` Junio C Hamano
2009-07-25 17:41 ` Johannes Schindelin
1 sibling, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2009-07-25 17:39 UTC (permalink / raw)
To: Alex Vandiver; +Cc: Johannes Schindelin, git
Alex Vandiver <alex@chmrr.net> writes:
> At Sat Jul 25 10:09:56 -0400 2009, Johannes Schindelin wrote:
>> Is this not unnecessary, given that we only call that function when we
>> know that buf[0] == '[':
>
> Yes. However, given that I had changed the calling convention for the
> code, (it used to be passed the string starting just _inside_ the
> '['), I wanted to make the new calling convention clearer, and catch
> any places that were using the old convention.
It's Ok. I do not think this is performance critical part of the system,
and I'd feel safer with a bit of defensive programming like this,
especially because "git config" that writes (reader is Ok) has
traditionally been one of the most fragile part of the system.
> I'm happy to submit a new version without it, if you wish.
>
>> I was a bit surprised that "offset" is not used further in your patch, but
>> I saw that 2/2 uses it.
>
> Yeah, this hunk should probably have gone in 2/2 instead.
I actually thought about suggesting to squash these two patches into one,
as the change in [1/2] only makes sense in the context of the [2/2], but
decided against it.
I haven't applied (actually, I didn't even notice until this morning) the
small documentation update from Nana. Have any comments on that one?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] Make section_name_match start on '[', and return the length on success
2009-07-25 17:18 ` Alex Vandiver
2009-07-25 17:39 ` Junio C Hamano
@ 2009-07-25 17:41 ` Johannes Schindelin
1 sibling, 0 replies; 17+ messages in thread
From: Johannes Schindelin @ 2009-07-25 17:41 UTC (permalink / raw)
To: Alex Vandiver; +Cc: git
Hi,
On Sat, 25 Jul 2009, Alex Vandiver wrote:
> At Sat Jul 25 10:09:56 -0400 2009, Johannes Schindelin wrote:
> > Is this not unnecessary, given that we only call that function when we
> > know that buf[0] == '[':
>
> Yes. However, given that I had changed the calling convention for the
> code, (it used to be passed the string starting just _inside_ the
> '['), I wanted to make the new calling convention clearer, and catch
> any places that were using the old convention.
>
> I'm happy to submit a new version without it, if you wish.
>
> > I was a bit surprised that "offset" is not used further in your patch, but
> > I saw that 2/2 uses it.
>
> Yeah, this hunk should probably have gone in 2/2 instead.
No need to change anything, Junio already applied your patches.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2009-07-26 20:24 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-07-24 21:21 [PATCH 0/2] Section renaming can lose content Alex Vandiver
2009-07-24 21:21 ` [PATCH 1/2] Make section_name_match start on '[', and return the length on success Alex Vandiver
2009-07-24 21:21 ` [PATCH 2/2] After renaming a section, print any trailing variable definitions Alex Vandiver
2009-07-24 22:11 ` Nanako Shiraishi
2009-07-24 22:26 ` Alex Vandiver
2009-07-26 16:18 ` [PATCH] Make git config fail on variables with no section, as documented Alex Vandiver
2009-07-26 16:49 ` Johannes Schindelin
2009-07-26 19:32 ` Alex Vandiver
2009-07-26 19:48 ` Johannes Schindelin
2009-07-26 20:24 ` Alex Vandiver
2009-07-24 23:39 ` [PATCH 2/2] After renaming a section, print any trailing variable definitions Junio C Hamano
2009-07-25 0:28 ` (unknown), Nanako Shiraishi
2009-07-25 14:10 ` [PATCH 2/2] After renaming a section, print any trailing variable definitions Johannes Schindelin
2009-07-25 14:09 ` [PATCH 1/2] Make section_name_match start on '[', and return the length on success Johannes Schindelin
2009-07-25 17:18 ` Alex Vandiver
2009-07-25 17:39 ` Junio C Hamano
2009-07-25 17:41 ` Johannes Schindelin
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).