Extract candidate identification

px/px_commandline.py

@@ -19,6 +19,31 @@
 PERL_BIN = re.compile("^perl[.0-9]*$")
 
 
+def get_coalesce_candidate(so_far: str) -> Optional[str]:
+    start_index = -1
+    if so_far.startswith("/"):
+        start_index = 0
+    elif so_far.startswith("-"):
+        equals_slash_index = so_far.find("=/")
+        if equals_slash_index == -1:
+            # No =/ in the string, so we're not looking at -Djava.io.tmpdir=/tmp
+            return None
+
+        # Start at the slash after the equals sign
+        start_index = equals_slash_index + 1
+
+    if start_index == -1:
+        # Start not found, this is not a path
+        return None
+
+    colon_slash_index = so_far.rfind(":/", start_index)
+    if colon_slash_index == -1:
+        # Not a : separated path
+        return so_far[start_index:]
+
+    return so_far[colon_slash_index + 1 :]
+
+
 def should_coalesce(
     parts: List[str], exists: Callable[[str], bool] = os.path.exists
 ) -> Optional[bool]:
@@ -28,68 +53,40 @@ def should_coalesce(
     path, or a : separated series of file paths.
 
     Return values:
-    * True: Coalesce
-    * False: Do not coalesce, leave these parts alone
-    * None: Do not coalesce, but if you add more parts then that might work
+    * True: Coalesce, done
+    * False: Do not coalesce, done
+    * None: Undecided, add another part and try again
     """
 
-    if parts[-1].startswith("-"):
-        # "xxx -yyy" would make no sense coalesced, that - likely means what
-        # comes after is a command line switch
-        return False
-
-    if parts[-1].startswith("/"):
-        # "xxx /yyy" would make no sense coalesced
+    if parts[-1].startswith("-") or parts[-1].startswith("/"):
+        # Last part starts a command line option or a new absolute path, don't
+        # coalesce.
         return False
 
     coalesced = " ".join(parts)
-
-    # Find the last possible starting point of an absolute path. This would be
-    # either a leading /, or a : or a = followed by a /.
-    path_start_index = -1
-
-    if coalesced.startswith("/"):
-        # /x/y/z
-        path_start_index = 0
-    if (first_equals_slash := coalesced.find("=/")) >= 0:
-        # -Dhello=/x/y/z
-        path_start_index = first_equals_slash + 1
-    if (last_colon_slash := coalesced.rfind(":/")) >= 0:
-        if last_colon_slash > path_start_index:
-            # -Dsomepath=/a/b/c:/x/y/z
-            path_start_index = last_colon_slash + 1
-
-    if path_start_index == -1:
-        # Absolute path not found, do not coalesce
+    candidate = get_coalesce_candidate(coalesced)
+    if not candidate:
+        # This is not a candidate for coalescing
         return False
 
-    path_end_index_exclusive = len(coalesced)
-    if (last_colon := coalesced.rfind(":")) >= 0:
-        if last_colon > path_start_index:
-            path_end_index_exclusive = last_colon
-
-    candidate = coalesced[path_start_index:path_end_index_exclusive]
-    candidate = candidate.removesuffix("/")  # Simplifies testing
     if exists(candidate):
-        if path_end_index_exclusive == len(coalesced):
-            # End of input exists, coalesce
-            return True
-        else:
-            # Candidate exists, but input continues, keep looking
-            return None
-    else:
-        # Candidate does not exist, but maybe it's incomplete, keep looking
+        # Found it, done!
+        return True
+
+    if exists(os.path.dirname(candidate)):
+        # Found the parent directory, we're on the right track, keep looking!
         return None
 
+    # Candidate does not exists, and neither does its parent directory, this is
+    # not it.
+    return False
+
 
 def coalesce_count(
     parts: List[str], exists: Callable[[str], bool] = os.path.exists
 ) -> int:
     """How many parts should be coalesced?"""
 
-    # This is what we can be sure of so far
-    confirmed_count = 1
-
     for coalesce_count in range(2, len(parts) + 1):
         should_coalesce_ = should_coalesce(parts[0:coalesce_count], exists)
 
@@ -98,13 +95,13 @@ def coalesce_count(
             continue
 
         if should_coalesce_ is False:
-            return confirmed_count
+            return 1
 
         # should_coalesce_ is True
-        confirmed_count = coalesce_count
+        return coalesce_count
 
     # Undecided until the end, this means no coalescing should be done
-    return confirmed_count
+    return 1
 
 
 def to_array(

tests/px_commandline_test.py

@@ -3,60 +3,27 @@
 from px import px_commandline
 
 
-def test_should_coalesce():
-    def exists(s):
-        return s in [
-            "/Applications",
-            "/Applications/IntelliJ IDEA.app",
-        ]
+def test_get_coalesce_candidate():
+    assert px_commandline.get_coalesce_candidate("/hello") == "/hello"
+    assert px_commandline.get_coalesce_candidate("/hello:/baloo") == "/baloo"
 
-    assert not px_commandline.should_coalesce(
-        ["java", "-Dhello=/Applications/IntelliJ"], exists=exists
-    )
+    assert px_commandline.get_coalesce_candidate("-Dx=/hello") == "/hello"
+    assert px_commandline.get_coalesce_candidate("-Dx=/hello:/baloo") == "/baloo"
 
-    assert (
-        px_commandline.should_coalesce(
-            ["-Dhello=/Applications/IntelliJ", "IDEA.app/Contents"], exists=exists
-        )
-        is None  # Potentially incomplete
-    )
-
-    assert (
-        px_commandline.should_coalesce(
-            [
-                "/Applications/IntelliJ",
-                "IDEA.app/Contents/plugins/maven-model/lib/maven-model.jar:/Applications/IntelliJ",
-            ],
-            exists=exists,
-        )
-        is None  # Potentially incomplete
-    )
+    assert px_commandline.get_coalesce_candidate("hello") is None
+    assert px_commandline.get_coalesce_candidate("hello:/baloo") is None
 
     assert (
-        px_commandline.should_coalesce(
-            [
-                "/Applications/IntelliJ IDEA.app/Contents/plugins/maven-model/lib/maven-model.jar:/Applications/IntelliJ",
-                "IDEA.app/Contents/plugins/maven-server/lib/maven-server.jar",
-            ],
-            exists=exists,
+        px_commandline.get_coalesce_candidate(
+            "/A/IntelliJ IDEA.app/C/p/mm/lib/mm.jar:/A/IntelliJ IDEA.app/C/p/ms/lib/ms.jar:/A/IntelliJ"
         )
-        is None  # Potentially incomplete
-    )
-
-    assert px_commandline.should_coalesce(
-        ["/A/MS Edge.app/MS Edge"],
-        exists=lambda s: s in ["/A/MS Edge.app", "/A/MS Edge.app/MS Edge"],
-    )
-
-    assert px_commandline.should_coalesce(
-        ["/A/MS Edge.app/MS Edge:/A/MS Edge.app/MS Edge"],
-        exists=lambda s: s in ["/A/MS Edge.app", "/A/MS Edge.app/MS Edge"],
+        == "/A/IntelliJ"
     )
 
 
 def test_coalesce_count():
     def exists(s):
-        return s == "/a b c"
+        return s in ["/", "/a b c", "/a b c/"]
 
     assert px_commandline.coalesce_count(["/a", "b", "c"], exists=exists) == 3
     assert px_commandline.coalesce_count(["/a", "b", "c/"], exists=exists) == 3
@@ -110,6 +77,7 @@ def test_to_array_spaced2():
         ),
         exists=lambda s: s
         in [
+            "/Applications",
             "/Applications/IntelliJ IDEA.app/Contents/Info.plist",
             "/Applications/IntelliJ IDEA.app/Contents/plugins/maven-model/lib/maven-model.jar",
             "/Applications/IntelliJ IDEA.app/Contents/plugins/maven-server/lib/maven-server.jar",
@@ -144,6 +112,7 @@ def test_to_array_spaced3():
         ),
         exists=lambda s: s
         in [
+            "/Applications",
             "/Applications/IntelliJ IDEA CE.app/Contents/Info.plist",
             "/Applications/IntelliJ IDEA CE.app/Contents/plugins/maven-model/lib/maven-model.jar",
             "/Applications/IntelliJ IDEA CE.app/Contents/plugins/maven-server/lib/maven-server.jar",