Config.removeSection() telling whether it changed the config

Add a variant of unsetSection() that returns whether it did indeed
change the config. This can be used in to skip saving the config if
it was not changed.

Also fix the iteration over the entries: lastWasMatch was never reset,
and thus all empty lines after a match would be removed.

Change-Id: Iea9e84aa74b1e4bb3c89efe3936fa3a8a09532e5
Signed-off-by: Thomas Wolf <twolf@apache.org>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java
index 36cf77bb018a942d1a8041e3453b1226693199b7..0c0257df9002635f6d5375e5ae253b7c5e75c6f3 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java
@@ -506,6 +506,35 @@ public class ConfigTest {
                assertEquals("[my]\n\tempty\n", c.toText());
        }
 
+       @Test
+       public void testRemoveBranchSection() throws ConfigInvalidException {
+               Config c = parse("" //
+                               + "[branch \"keep\"]\n"
+                               + "  merge = master.branch.to.keep.in.the.file\n"
+                               + "\n"
+                               + "[branch \"remove\"]\n"
+                               + "  merge = this.will.get.deleted\n"
+                               + "  remote = origin-for-some-long-gone-place\n"
+                               + "\n"
+                               + "\n"
+                               + "[core-section-not-to-remove-in-test]\n"
+                               + "  packedGitLimit = 14\n"
+                               + "\n"
+                               + "[other]\n"
+                               + "  foo = bar\n");
+               assertFalse(c.removeSection("branch", "does.not.exist"));
+               assertTrue(c.removeSection("branch", "remove"));
+               assertEquals("" //
+                               + "[branch \"keep\"]\n"
+                               + "  merge = master.branch.to.keep.in.the.file\n"
+                               + "\n"
+                               + "[core-section-not-to-remove-in-test]\n"
+                               + "  packedGitLimit = 14\n"
+                               + "\n"
+                               + "[other]\n"
+                               + "  foo = bar\n", c.toText());
+       }
+
        @Test
        public void testUnsetBranchSection() throws ConfigInvalidException {
                Config c = parse("" //
@@ -516,8 +545,12 @@ public class ConfigTest {
                                + "  merge = this.will.get.deleted\n"
                                + "  remote = origin-for-some-long-gone-place\n"
                                + "\n"
+                               + "\n"
                                + "[core-section-not-to-remove-in-test]\n"
-                               + "  packedGitLimit = 14\n");
+                               + "  packedGitLimit = 14\n"
+                               + "\n"
+                               + "[other]\n"
+                               + "  foo = bar\n");
                c.unsetSection("branch", "does.not.exist");
                c.unsetSection("branch", "remove");
                assertEquals("" //
@@ -525,7 +558,10 @@ public class ConfigTest {
                                + "  merge = master.branch.to.keep.in.the.file\n"
                                + "\n"
                                + "[core-section-not-to-remove-in-test]\n"
-                               + "  packedGitLimit = 14\n", c.toText());
+                               + "  packedGitLimit = 14\n"
+                               + "\n"
+                               + "[other]\n"
+                               + "  foo = bar\n", c.toText());
        }
 
        @Test

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java
index 7e2c5b5ad0c5dac372209cd47bb10071fd805f8b..07c5fa450075810fcdfa24ae9cc11142a8547cce 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java
@@ -30,6 +30,7 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.eclipse.jgit.annotations.NonNull;
@@ -921,29 +922,52 @@ public class Config {
         *            optional subsection value, e.g. a branch name
         */
        public void unsetSection(String section, String subsection) {
+               removeSection(section, subsection);
+       }
+
+       /**
+        * Removes all configuration values under a single section.
+        *
+        * @param section
+        *            section name, e.g "branch"
+        * @param subsection
+        *            optional subsection value, e.g. a branch name
+        * @return {@code true} if a section was present and was removed;
+        *         {@code false} if the config was not changed (i.e., no such
+        *         section was present)
+        * @since 6.8
+        */
+       public boolean removeSection(String section, String subsection) {
                ConfigSnapshot src, res;
+               AtomicBoolean changed = new AtomicBoolean();
                do {
                        src = state.get();
-                       res = unsetSection(src, section, subsection);
+                       changed.set(false);
+                       res = unsetSection(src, section, subsection, changed);
                } while (!state.compareAndSet(src, res));
+               return changed.get();
        }
 
-       private ConfigSnapshot unsetSection(final ConfigSnapshot srcState,
-                       final String section,
-                       final String subsection) {
+       private ConfigSnapshot unsetSection(ConfigSnapshot srcState, String section,
+                       String subsection, AtomicBoolean changed) {
                final int max = srcState.entryList.size();
                final ArrayList<ConfigLine> r = new ArrayList<>(max);
 
                boolean lastWasMatch = false;
                for (ConfigLine e : srcState.entryList) {
-                       if (e.includedFrom == null && e.match(section, subsection)) {
-                               // Skip this record, it's for the section we are removing.
-                               lastWasMatch = true;
+                       if (e.includedFrom != null) {
+                               r.add(e);
                                continue;
                        }
-
-                       if (lastWasMatch && e.section == null && e.subsection == null)
+                       if (lastWasMatch && e.section == null && e.subsection == null) {
                                continue; // skip this padding line in the section.
+                       }
+                       lastWasMatch = e.match(section, subsection);
+                       if (lastWasMatch) {
+                               // Skip this record, it's for the section we are removing.
+                               changed.set(true);
+                               continue;
+                       }
                        r.add(e);
                }
 

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java
index 7fdcc4d3ecbffd5d79bd0efc8f628a77a9de4107..cedc4d6cc9074c97252b0b0609d7d8ff43c2b12f 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java
@@ -5,7 +5,7 @@
  * Copyright (C) 2009, JetBrains s.r.o.
  * Copyright (C) 2008-2009, Robin Rosenberg <robin.rosenberg@dewire.com>
  * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
- * Copyright (C) 2008, Thad Hughes <thadh@thad.corp.google.com> and others
+ * Copyright (C) 2008, 2023 Thad Hughes <thadh@thad.corp.google.com> and others
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Distribution License v. 1.0 which is available at
@@ -22,7 +22,6 @@ import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
 import java.text.MessageFormat;
-import java.util.List;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -107,17 +106,6 @@ public class FileBasedConfig extends StoredConfig {
                return exists.get();
        }
 
-       @Override
-       public void setStringList(String section, String subsection, String name,
-                       List<String> values) {
-               super.setStringList(section, subsection, name, values);
-       }
-
-       @Override
-       public void unsetSection(String section, String subsection) {
-               super.unsetSection(section, subsection);
-       }
-
        /**
         * {@inheritDoc}
         * <p>

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/UserConfigFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/UserConfigFile.java
index 2ad74c23c7648f1aeca8fa5c817537d86db70853..c1eaac7b55ee3c554cece1e8ca13c2e1e5cf4c02 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/UserConfigFile.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/UserConfigFile.java
@@ -82,6 +82,14 @@ public class UserConfigFile extends FileBasedConfig {
                }
        }
 
+       @Override
+       public boolean removeSection(String section, String subsection) {
+               if (exists() || !parent.exists()) {
+                       return super.removeSection(section, subsection);
+               }
+               return parent.removeSection(section, subsection);
+       }
+
        @Override
        public boolean isOutdated() {
                return super.isOutdated() || parent.isOutdated();