@matheusbsbs / matheus-bagliano / commits / 22aedfccd0

git config --unset: remove empty sections (in the common case)

The original reasoning for not removing section headers upon removal of the last entry went like this: the user could have added comments about the section, or about the entries therein, and if there were other comments there, we would not know whether we should remove them. In particular, a concocted example was presented that looked like this (and was added to t1300): # some generic comment on the configuration file itself # a comment specific to this "section" section. [section] # some intervening lines # that should also be dropped key = value # please be careful when you update the above variable The ideal thing for `git config --unset section.key` in this case would be to leave only the first line behind, because all the other comments are now obsolete. However, this is unfeasible, short of adding a complete Natural Language Processing module to Git, which seems not only a lot of work, but a totally unreasonable feature (for little benefit to most users). Now, the real kicker about this problem is: most users do not edit their config files at all! In their use case, the config looks like this instead: [section] key = value ... and it is totally obvious what should happen if the entry is removed: the entire section should vanish. Let's generalize this observation to this conservative strategy: if we are removing the last entry from a section, and there are no comments inside that section nor surrounding it, then remove the entire section. Otherwise behave as before: leave the now-empty section (including those comments, even ones about the now-deleted entry). We have to be extra careful to handle the case where more than one entry is removed: any subset of them might be the last entries of their respective sections (and if there are no comments in or around that section, the section should be removed, too). 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 22aedfccd0cfe7c8f09a990cf4777efc7f27bd2e

2 files changed +93 -4

config.c

+91 -2

@@ -2297,6 +2297,7 @@ struct config_store_data {

2297 struct {

2298 size_t begin, end;

2299 enum config_event_t type;

2300 + int is_keys_section;

2301 } *parsed;

2302 unsigned int parsed_nr, parsed_alloc, *seen, seen_nr, seen_alloc;

2303 unsigned int key_seen:1, section_seen:1, is_keys_section:1;

@@ -2325,17 +2326,20 @@ static int store_aux_event(enum config_event_t type,

2326 store->parsed[store->parsed_nr].begin = begin;

2327 store->parsed[store->parsed_nr].end = end;

2328 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 &&

2335 + store->is_keys_section =

2336 + store->parsed[store->parsed_nr].is_keys_section =

2337 + cf->var.len - 1 == store->baselen &&

2338 !strncasecmp(cf->var.buf, store->key, store->baselen);

2339 }

2340

2341 + store->parsed_nr++;

2342 +

2343 return 0;

2344 }

2345

@@ -2467,6 +2471,87 @@ static ssize_t write_pair(int fd, const char *key, const char *value,

2471 return ret;

2472 }

2473

2474 +/*

2475 + * If we are about to unset the last key(s) in a section, and if there are

2476 + * no comments surrounding (or included in) the section, we will want to

2477 + * extend begin/end to remove the entire section.

2478 + *

2479 + * Note: the parameter `seen_ptr` points to the index into the store.seen

2480 + * array. * This index may be incremented if a section has more than one

2481 + * entry (which all are to be removed).

2482 + */

2483 +static void maybe_remove_section(struct config_store_data *store,

2484 + const char *contents,

2485 + size_t *begin_offset, size_t *end_offset,

2486 + int *seen_ptr)

2487 +{

2488 + size_t begin;

2489 + int i, seen, section_seen = 0;

2490 +

2491 + /*

2492 + * First, ensure that this is the first key, and that there are no

2493 + * comments before the entry nor before the section header.

2494 + */

2495 + seen = *seen_ptr;

2496 + for (i = store->seen[seen]; i > 0; i--) {

2497 + enum config_event_t type = store->parsed[i - 1].type;

2498 +

2499 + if (type == CONFIG_EVENT_COMMENT)

2500 + /* There is a comment before this entry or section */

2501 + return;

2502 + if (type == CONFIG_EVENT_ENTRY) {

2503 + if (!section_seen)

2504 + /* This is not the section's first entry. */

2505 + return;

2506 + /* We encountered no comment before the section. */

2507 + break;

2508 + }

2509 + if (type == CONFIG_EVENT_SECTION) {

2510 + if (!store->parsed[i - 1].is_keys_section)

2511 + break;

2512 + section_seen = 1;

2513 + }

2514 + }

2515 + begin = store->parsed[i].begin;

2516 +

2517 + /*

2518 + * Next, make sure that we are removing he last key(s) in the section,

2519 + * and that there are no comments that are possibly about the current

2520 + * section.

2521 + */

2522 + for (i = store->seen[seen] + 1; i < store->parsed_nr; i++) {

2523 + enum config_event_t type = store->parsed[i].type;

2524 +

2525 + if (type == CONFIG_EVENT_COMMENT)

2526 + return;

2527 + if (type == CONFIG_EVENT_SECTION) {

2528 + if (store->parsed[i].is_keys_section)

2529 + continue;

2530 + break;

2531 + }

2532 + if (type == CONFIG_EVENT_ENTRY) {

2533 + if (++seen < store->seen_nr &&

2534 + i == store->seen[seen])

2535 + /* We want to remove this entry, too */

2536 + continue;

2537 + /* There is another entry in this section. */

2538 + return;

2539 + }

2540 + }

2541 +

2542 + /*

2543 + * We are really removing the last entry/entries from this section, and

2544 + * there are no enclosed or surrounding comments. Remove the entire,

2545 + * now-empty section.

2546 + */

2547 + *seen_ptr = seen;

2548 + *begin_offset = begin;

2549 + if (i < store->parsed_nr)

2550 + *end_offset = store->parsed[i].begin;

2551 + else

2552 + *end_offset = store->parsed[store->parsed_nr - 1].end;

2553 +}

2554 +

2555 int git_config_set_in_file_gently(const char *config_filename,

2556 const char *key, const char *value)

2557 {

@@ -2689,6 +2774,10 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,

2774 } else {

2775 replace_end = store.parsed[j].end;

2776 copy_end = store.parsed[j].begin;

2777 + if (!value)

2778 + maybe_remove_section(&store, contents,

2779 + &copy_end,

2780 + &replace_end, &i);

2781 /*

2782 * Swallow preceding white-space on the same

2783 * line.

t/t1300-config.sh

+2 -2

@@ -1390,7 +1390,7 @@ test_expect_success 'urlmatch with wildcard' '

1390 '

1391

1392 # good section hygiene

1393 -test_expect_failure '--unset last key removes section (except if commented)' '

1393 +test_expect_success '--unset last key removes section (except if commented)' '

1394 cat >.git/config <<-\EOF &&

1395 # some generic comment on the configuration file itself

1396 # a comment specific to this "section" section.

@@ -1472,7 +1472,7 @@ test_expect_failure '--unset last key removes section (except if commented)' '

1472 test_line_count = 3 .git/config

1473 '

1474

1475 -test_expect_failure '--unset-all removes section if empty & uncommented' '

1475 +test_expect_success '--unset-all removes section if empty & uncommented' '

1476 cat >.git/config <<-\EOF &&

1477 [section]

1478 key = value1