Michael J Gruber [off-list ref] writes:
Junio C Hamano venit, vidit, dixit 14.08.2009 21:52:
quoted
Michael J Gruber [off-list ref] writes:
...
But I may be being overly cautious.
My first version had the lenient mode for "git config -e" only, which
required a new global int (or, alternatively, changing all callers).
Without looking at the actual patch, that "single global that is only used
by builtin_config() when using -e" sounds safer.
But still I think I may be being overly cautious.
quoted
By the way, why did you have to change s/echo/printf/? Can't you give two
lines in a single argument without "\n" escape?
Because "printf" is more portable then "echo -e". At least I hope so ;)
[ One could use a "here document", of course. Is that preferable? ]
What I meant was to give literally two lines, like this:
check section.key 'warning: bad config file line 2 in .git/config
warning: bad config file line 2 in .git/config'
I do not see a need for a here-doc.
The rest is tangent you can ignore.
quoted
quoted
Instead, issue a warning only and treat the rest of the line as a
comment (ignore it). This benefits not only git config -e users.
... a broken sentence in the middle? I would have expected the "not only"
followed by "but also"; the question is "but also what?"
I don't see any broken sentence here. "Benefit" is a verb as well as a noun.
Yes, and I think you read me correctly.
s/broken/chopped in the middle, missing 'but also X'/;
and you just explained that you meant "everyone else" by that X in the
missing part of the sentence.
Currently, git config dies as soon as there is a parsing error. This is
especially unfortunate in case a user tries to correct config mistakes
using git config -e.
Instead, issue a warning only and treat the rest of the line as a
comment (ignore it). This benefits not only git config -e users but
also everyone else.
Signed-off-by: Michael J Gruber <redacted>
Reported-by: David Reitter <redacted>
---
config.c | 80 ++++++++++++++++++++++++----------------------
t/t1303-wacky-config.sh | 3 +-
2 files changed, 44 insertions(+), 39 deletions(-)
So, after a business trip, vacation, ... I'm finally returning to this patch.
I addressed the echo/printf issue as suggested and clarified the commit message.
Regarding the global int for switching on/off lenient parsing: I reinstated my
v0 of the patch only to find out (again) that setup_git_directory_gently is the
problem here. We would have to turn on lenient parsing before we even know that
"-e" is supplied. So, the only option would be to have "git config" use lenient
parsing in all modes (edit/get/set) and have other git commands die fatally on
erroneous config.
So, here's v2 which has lenient config parsing for everyone because I don't see
a way to have it for "git config -e" only. If you prefer to have it for all of
"git config" only, that version is in another branch head now...
Cheers,
Michael
diff --git a/config.c b/config.c
index e87edea..5e0af5d 100644
--- a/config.c
+++ b/config.c
@@ -207,50 +207,54 @@ static int git_parse_file(config_fn_t fn, void *data)
static const unsigned char *utf8_bom = (unsigned char *) "\xef\xbb\xbf";
const unsigned char *bomptr = utf8_bom;
- for (;;) {
- int c = get_next_char();
- if (bomptr && *bomptr) {
- /* We are at the file beginning; skip UTF8-encoded BOM
- * if present. Sane editors won't put this in on their
- * own, but e.g. Windows Notepad will do it happily. */
- if ((unsigned char) c == *bomptr) {
- bomptr++;
+ while (!config_file_eof) {
+ for (;;) {
+ int c = get_next_char();
+ if (bomptr && *bomptr) {
+ /* We are at the file beginning; skip UTF8-encoded BOM
+ * if present. Sane editors won't put this in on their
+ * own, but e.g. Windows Notepad will do it happily. */
+ if ((unsigned char) c == *bomptr) {
+ bomptr++;
+ continue;
+ } else {
+ /* Do not tolerate partial BOM. */
+ if (bomptr != utf8_bom)
+ break;
+ /* No BOM at file beginning. Cool. */
+ bomptr = NULL;
+ }
+ }
+ if (c == '\n') {
+ if (config_file_eof)
+ return 0;
+ comment = 0;
continue;
- } else {
- /* Do not tolerate partial BOM. */
- if (bomptr != utf8_bom)
+ }
+ if (comment || isspace(c))
+ continue;
+ if (c == '#' || c == ';') {
+ comment = 1;
+ continue;
+ }
+ if (c == '[') {
+ baselen = get_base_var(var);
+ if (baselen <= 0)
break;
- /* No BOM at file beginning. Cool. */
- bomptr = NULL;
+ var[baselen++] = '.';
+ var[baselen] = 0;
+ continue;
}
- }
- if (c == '\n') {
- if (config_file_eof)
- return 0;
- comment = 0;
- continue;
- }
- if (comment || isspace(c))
- continue;
- if (c == '#' || c == ';') {
- comment = 1;
- continue;
- }
- if (c == '[') {
- baselen = get_base_var(var);
- if (baselen <= 0)
+ if (!isalpha(c))
+ break;
+ var[baselen] = tolower(c);
+ if (get_value(fn, data, var, baselen+1) < 0)
break;
- var[baselen++] = '.';
- var[baselen] = 0;
- continue;
}
- if (!isalpha(c))
- break;
- var[baselen] = tolower(c);
- if (get_value(fn, data, var, baselen+1) < 0)
- break;
+ warning("bad config file line %d in %s", config_linenr, config_file_name);
+ comment = 1;
}
- die("bad config file line %d in %s", config_linenr, config_file_name);
+ return -1;
}
static int parse_unit_factor(const char *end, unsigned long *val)diff --git a/t/t1303-wacky-config.sh b/t/t1303-wacky-config.sh
index 080117c..0599d9f 100755
--- a/t/t1303-wacky-config.sh
+++ b/t/t1303-wacky-config.sh
@@ -44,7 +44,8 @@ LONG_VALUE=$(printf "x%01021dx a" 7)
test_expect_success 'do not crash on special long config line' '
setup &&
git config section.key "$LONG_VALUE" &&
- check section.key "fatal: bad config file line 2 in .git/config"
+ check section.key "warning: bad config file line 2 in .git/config
+warning: bad config file line 2 in .git/config"
'
test_done
--
1.6.4.2.395.ge3d52