git_config_set: make use of the config parser's event stream

In the recent commit with the title "config: introduce an optional event stream while parsing", we introduced an optional callback to keep track of the config parser's events "comment", "white-space", "section header" and "entry". One motivation for this feature was to make use of it in the code that edits the config. And this commit makes it so. Note: this patch changes the meaning of the `seen` array that records whether we saw the config entry that is to be edited: previously, it contained the end offset of the found entry. Now, we introduce a new array `parsed` that keeps a record of *all* config parser events (with begin/end offsets), and the items in the `seen` array now point into the `parsed` array. There are two reasons why we do it this way: 1. To keep the implementation simple, the config parser's event stream reports the event only after the config callback was called, so we would not receive the begin offset otherwise. 2. In the following patches, we will re-use the `parsed` array to fix two long-standing bugs related to empty sections. Note that this also makes the code more robust with respect to finding the begin offset of the part(s) of the config file to be edited, as we no longer back-track to find the beginning of the line. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Johannes Schindelin committed Apr 9, 2018 at 10:32 UTC 6ae996f2acf3ad780b8d338c81e24143f0b0d304
1 file changed +81 -89
config.c
+81 -89
@@ -2294,8 +2294,11 @@ struct config_store_data {
2294 int do_not_match;
2295 regex_t *value_regex;
2296 int multi_replace;
2297 - size_t *seen;
2298 - unsigned int seen_nr, seen_alloc;
2297 + struct {
2298 + size_t begin, end;
2299 + enum config_event_t type;
2300 + } *parsed;
2301 + unsigned int parsed_nr, parsed_alloc, *seen, seen_nr, seen_alloc;
2302 unsigned int key_seen:1, section_seen:1, is_keys_section:1;
2303 };
2304
@@ -2313,10 +2316,31 @@ static int matches(const char *key, const char *value,
2316 (value && !regexec(store->value_regex, value, 0, NULL, 0));
2317 }
2318
2319 +static int store_aux_event(enum config_event_t type,
2320 + size_t begin, size_t end, void *data)
2321 +{
2322 + struct config_store_data *store = data;
2323 +
2324 + ALLOC_GROW(store->parsed, store->parsed_nr + 1, store->parsed_alloc);
2325 + store->parsed[store->parsed_nr].begin = begin;
2326 + store->parsed[store->parsed_nr].end = end;
2327 + store->parsed[store->parsed_nr].type = type;
2328 + store->parsed_nr++;
2329 +
2330 + if (type == CONFIG_EVENT_SECTION) {
2331 + if (cf->var.len < 2 || cf->var.buf[cf->var.len - 1] != '.')
2332 + BUG("Invalid section name '%s'", cf->var.buf);
2333 +
2334 + /* Is this the section we were looking for? */
2335 + store->is_keys_section = cf->var.len - 1 == store->baselen &&
2336 + !strncasecmp(cf->var.buf, store->key, store->baselen);
2337 + }
2338 +
2339 + return 0;
2340 +}
2341 +
2342 static int store_aux(const char *key, const char *value, void *cb)
2343 {
2318 - const char *ep;
2319 - size_t section_len;
2344 struct config_store_data *store = cb;
2345
2346 if (store->key_seen) {
@@ -2328,55 +2352,21 @@ static int store_aux(const char *key, const char *value, void *cb)
2352 ALLOC_GROW(store->seen, store->seen_nr + 1,
2353 store->seen_alloc);
2354
2331 - store->seen[store->seen_nr] = cf->do_ftell(cf);
2355 + store->seen[store->seen_nr] = store->parsed_nr;
2356 store->seen_nr++;
2357 }
2334 - return 0;
2358 } else if (store->is_keys_section) {
2359 /*
2337 - * What we are looking for is in store->key (both
2338 - * section and var), and its section part is baselen
2339 - * long. We found key (again, both section and var).
2340 - * We would want to know if this key is in the same
2341 - * section as what we are looking for. We already
2342 - * know we are in the same section as what should
2343 - * hold store->key.
2360 + * Do not increment matches yet: this may not be a match, but we
2361 + * are in the desired section.
2362 */
2345 - ep = strrchr(key, '.');
2346 - section_len = ep - key;
2347 -
2348 - if ((section_len != store->baselen) ||
2349 - memcmp(key, store->key, section_len+1)) {
2350 - store->is_keys_section = 0;
2351 - return 0;
2352 - }
2353 - /*
2354 - * Do not increment matches: this is no match, but we
2355 - * just made sure we are in the desired section.
2356 - */
2357 - ALLOC_GROW(store->seen, store->seen_nr + 1,
2358 - store->seen_alloc);
2359 - store->seen[store->seen_nr] = cf->do_ftell(cf);
2360 - }
2361 -
2362 - if (matches(key, value, store)) {
2363 - ALLOC_GROW(store->seen, store->seen_nr + 1,
2364 - store->seen_alloc);
2365 - store->seen[store->seen_nr] = cf->do_ftell(cf);
2366 - store->seen_nr++;
2367 - store->key_seen = 1;
2363 + ALLOC_GROW(store->seen, store->seen_nr + 1, store->seen_alloc);
2364 + store->seen[store->seen_nr] = store->parsed_nr;
2365 store->section_seen = 1;
2369 - store->is_keys_section = 1;
2370 - } else {
2371 - if (strrchr(key, '.') - key == store->baselen &&
2372 - !strncmp(key, store->key, store->baselen)) {
2373 - store->section_seen = 1;
2374 - store->is_keys_section = 1;
2375 - ALLOC_GROW(store->seen,
2376 - store->seen_nr + 1,
2377 - store->seen_alloc);
2378 - store->seen[store->seen_nr] =
2379 - cf->do_ftell(cf);
2366 +
2367 + if (matches(key, value, store)) {
2368 + store->seen_nr++;
2369 + store->key_seen = 1;
2370 }
2371 }
2372
@@ -2477,32 +2467,6 @@ static ssize_t write_pair(int fd, const char *key, const char *value,
2467 return ret;
2468 }
2469
2480 -static ssize_t find_beginning_of_line(const char *contents, size_t size,
2481 - size_t offset_, int *found_bracket)
2482 -{
2483 - size_t equal_offset = size, bracket_offset = size;
2484 - ssize_t offset;
2485 -
2486 -contline:
2487 - for (offset = offset_-2; offset > 0
2488 - && contents[offset] != '\n'; offset--)
2489 - switch (contents[offset]) {
2490 - case '=': equal_offset = offset; break;
2491 - case ']': bracket_offset = offset; break;
2492 - }
2493 - if (offset > 0 && contents[offset-1] == '\\') {
2494 - offset_ = offset;
2495 - goto contline;
2496 - }
2497 - if (bracket_offset < equal_offset) {
2498 - *found_bracket = 1;
2499 - offset = bracket_offset+1;
2500 - } else
2501 - offset++;
2502 -
2503 - return offset;
2504 -}
2505 -
2470 int git_config_set_in_file_gently(const char *config_filename,
2471 const char *key, const char *value)
2472 {
@@ -2613,6 +2577,7 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
2577 struct stat st;
2578 size_t copy_begin, copy_end;
2579 int i, new_line = 0;
2580 + struct config_options opts;
2581
2582 if (value_regex == NULL)
2583 store.value_regex = NULL;
@@ -2635,17 +2600,24 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
2600 }
2601 }
2602
2638 - ALLOC_GROW(store.seen, 1, store.seen_alloc);
2639 - store.seen[0] = 0;
2640 - store.seen_nr = 0;
2603 + ALLOC_GROW(store.parsed, 1, store.parsed_alloc);
2604 + store.parsed[0].end = 0;
2605 +
2606 + memset(&opts, 0, sizeof(opts));
2607 + opts.event_fn = store_aux_event;
2608 + opts.event_fn_data = &store;
2609
2610 /*
2643 - * After this, store.offset will contain the *end* offset
2644 - * of the last match, or remain at 0 if no match was found.
2611 + * After this, store.parsed will contain offsets of all the
2612 + * parsed elements, and store.seen will contain a list of
2613 + * matches, as indices into store.parsed.
2614 + *
2615 * As a side effect, we make sure to transform only a valid
2616 * existing config file.
2617 */
2648 - if (git_config_from_file(store_aux, config_filename, &store)) {
2618 + if (git_config_from_file_with_options(store_aux,
2619 + config_filename,
2620 + &store, &opts)) {
2621 error("invalid config file %s", config_filename);
2622 free(store.key);
2623 if (store.value_regex != NULL &&
@@ -2697,19 +2669,39 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
2669 goto out_free;
2670 }
2671
2700 - if (store.seen_nr == 0)
2672 + if (store.seen_nr == 0) {
2673 + if (!store.seen_alloc) {
2674 + /* Did not see key nor section */
2675 + ALLOC_GROW(store.seen, 1, store.seen_alloc);
2676 + store.seen[0] = store.parsed_nr
2677 + - !!store.parsed_nr;
2678 + }
2679 store.seen_nr = 1;
2680 + }
2681
2682 for (i = 0, copy_begin = 0; i < store.seen_nr; i++) {
2683 + size_t replace_end;
2684 + int j = store.seen[i];
2685 +
2686 new_line = 0;
2705 - if (store.seen[i] == 0) {
2706 - store.seen[i] = copy_end = contents_sz;
2707 - } else if (!store.key_seen) {
2708 - copy_end = store.seen[i];
2709 - } else
2710 - copy_end = find_beginning_of_line(
2711 - contents, contents_sz,
2712 - store.seen[i], &new_line);
2687 + if (!store.key_seen) {
2688 + replace_end = copy_end = store.parsed[j].end;
2689 + } else {
2690 + replace_end = store.parsed[j].end;
2691 + copy_end = store.parsed[j].begin;
2692 + /*
2693 + * Swallow preceding white-space on the same
2694 + * line.
2695 + */
2696 + while (copy_end > 0 ) {
2697 + char c = contents[copy_end - 1];
2698 +
2699 + if (isspace(c) && c != '\n')
2700 + copy_end--;
2701 + else
2702 + break;
2703 + }
2704 + }
2705
2706 if (copy_end > 0 && contents[copy_end-1] != '\n')
2707 new_line = 1;
@@ -2723,7 +2715,7 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
2715 write_str_in_full(fd, "\n") < 0)
2716 goto write_err_out;
2717 }
2726 - copy_begin = store.seen[i];
2718 + copy_begin = replace_end;
2719 }
2720
2721 /* write the pair (value == NULL means unset) */