Merge branch 'johan/intellij-idea-ce' into python

Handle paths containing multiple spaces, like "IntelliJ IDEA CE.app".

px/px_commandline.py

@@ -20,55 +20,106 @@
 
 
 def should_coalesce(
-    part1: str, part2: str, exists: Callable[[str], bool] = os.path.exists
-) -> bool:
+    parts: List[str], exists: Callable[[str], bool] = os.path.exists
+) -> Optional[bool]:
     """
-    Two (previously) space separated command line parts should be coalesced if
-    combining them with a space in between creates an existing file path.
+    Two or more (previously) space separated command line parts should be
+    coalesced if combining them with a space in between creates an existing file
+    path.
+
+    Return values:
+    * True: Coalesce
+    * False: Do not coalesce. The first part here does not start a coalescable sequence.
+    * None: Do not coalesce, but if you add more parts then that might work.
     """
 
-    if part1.endswith("/"):
+    if parts[0].endswith("/"):
         # "xxx/ yyy" would make no sense coalesced
         return False
 
-    if part2.startswith("-"):
+    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 part2.startswith("/"):
+    if parts[-1].startswith("/"):
         # "xxx /yyy" would make no sense coalesced
         return False
 
     # Find the last possible starting point of an absolute path in part1
     path_start_index = -1
-    if part1.startswith("/"):
+    if parts[0].startswith("/"):
         # /x/y/z
         path_start_index = 0
-    if (first_equals_slash := part1.find("=/")) >= 0:
+    if (first_equals_slash := parts[0].find("=/")) >= 0:
         # -Dhello=/x/y/z
         path_start_index = first_equals_slash + 1
-    if (last_colon_slash := part1.rfind(":/")) >= 0:
+    if (last_colon_slash := parts[0].rfind(":/")) >= 0:
         if last_colon_slash > path_start_index:
             # -Dsomepath=/a/b/c:/x/y/z
             path_start_index = last_colon_slash + 1
 
-    # FIXME: Ignore (non-file:?) URLs?
-
     if path_start_index == -1:
-        # No path in part1, no need to coalesce
+        # Part 1 does not contain the start of any path, do not coalesce
         return False
 
-    path_end_index_exclusive = len(part2)
-    if (first_colon := part2.find(":")) >= 0:
+    path_end_index_exclusive = len(parts[-1])
+    if (first_colon := parts[-1].find(":")) >= 0:
         path_end_index_exclusive = first_colon
-    if (first_slash := part2.find("/")) >= 0:
+    if (first_slash := parts[-1].find("/")) >= 0:
         if first_slash < path_end_index_exclusive:
             path_end_index_exclusive = first_slash
 
-    candidate_path = part1[path_start_index:] + " " + part2[:path_end_index_exclusive]
+    middle = " "
+    if len(parts) > 2:
+        middle = " " + " ".join(parts[1:-1]) + " "
+
+    candidate_path = (
+        parts[0][path_start_index:] + middle + parts[-1][:path_end_index_exclusive]
+    )
+
+    _exists = exists(candidate_path)
+    if _exists:
+        return True
+    else:
+        if path_end_index_exclusive == len(parts[-1]):
+            # No hit, but no slashes in the final part, so we might still be
+            # inside of a multi-part name
+            return None
+        else:
+            # No hit, and we got something with slashes in it, so this is
+            # definitely a no.
+            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
+
+    section_start = 0
+    for coalesce_count in range(2, len(parts) + 1):
+        should_coalesce_ = should_coalesce(parts[section_start:coalesce_count], exists)
 
-    return exists(candidate_path)
+        if should_coalesce_ is None:
+            # Undecided, keep looking
+            continue
+
+        if should_coalesce_ is False:
+            return confirmed_count
+
+        # should_coalesce_ is True
+        confirmed_count = coalesce_count
+
+        # See if the last part should also be coalesced with what comes after
+        # it. Think of a search path for example: "/a b:/c d"
+        section_start = coalesce_count - 1
+
+    # Undecided until the end, this means no coalescing should be done
+    return confirmed_count
 
 
 def to_array(
@@ -80,12 +131,12 @@ def to_array(
         return base_split
 
     # Try to reverse engineer which spaces should not be split on
-    merged_split = [base_split[0]]
-    for part in base_split[1:]:
-        if should_coalesce(merged_split[-1], part, exists):
-            merged_split[-1] += " " + part
-        else:
-            merged_split.append(part)
+    merged_split: List[str] = []
+    i = 0
+    while i < len(base_split):
+        coalesce_count_ = coalesce_count(base_split[i:], exists)
+        merged_split.append(" ".join(base_split[i : i + coalesce_count_]))
+        i += coalesce_count_
 
     return merged_split
 

tests/px_commandline_test.py

@@ -11,26 +11,58 @@ def exists(s):
         ]
 
     assert not px_commandline.should_coalesce(
-        "java", "-Dhello=/Applications/IntelliJ", exists=exists
+        ["java", "-Dhello=/Applications/IntelliJ"], exists=exists
     )
 
     assert px_commandline.should_coalesce(
-        "-Dhello=/Applications/IntelliJ", "IDEA.app/Contents", exists=exists
+        ["-Dhello=/Applications/IntelliJ", "IDEA.app/Contents"], exists=exists
     )
 
     assert px_commandline.should_coalesce(
-        "/Applications/IntelliJ",
-        "IDEA.app/Contents/plugins/maven-model/lib/maven-model.jar:/Applications/IntelliJ",
+        [
+            "/Applications/IntelliJ",
+            "IDEA.app/Contents/plugins/maven-model/lib/maven-model.jar:/Applications/IntelliJ",
+        ],
         exists=exists,
     )
 
     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",
+        [
+            "/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,
     )
 
 
+def test_coalesce_count():
+    def exists(s):
+        return s == "/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
+    assert px_commandline.coalesce_count(["/a", "b", "c", "d"], exists=exists) == 3
+
+    assert (
+        px_commandline.coalesce_count(["/a", "b", "c:/a", "b", "c"], exists=exists) == 5
+    )
+    assert (
+        px_commandline.coalesce_count(["/a", "b", "c/:/a", "b", "c/"], exists=exists)
+        == 5
+    )
+
+    assert (
+        px_commandline.coalesce_count(["/a", "b", "c:/a", "b", "c", "d"], exists=exists)
+        == 5
+    )
+    assert (
+        px_commandline.coalesce_count(
+            ["/a", "b", "c/:/a", "b", "c/", "d/"], exists=exists
+        )
+        == 5
+    )
+
+
 def test_to_array_spaced1():
     assert px_commandline.to_array(
         "java -Dhello=/Applications/IntelliJ IDEA.app/Contents",
@@ -70,6 +102,38 @@ def test_to_array_spaced2():
     ]
 
 
+def test_to_array_spaced3():
+    """Same as test_to_array_spaced2() but with two spaces rather than just one."""
+    assert px_commandline.to_array(
+        " ".join(
+            [
+                "java",
+                "-Dhello=/Applications/IntelliJ IDEA CE.app/Contents/Info.plist",
+                "-classpath",
+                "/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:/Applications/IntelliJ",
+                "IDEA",
+                "CE.app/Contents/plugins/maven/lib/maven3-server-common.jar",
+                "MainClass",
+            ]
+        ),
+        exists=lambda s: s
+        in [
+            "/Applications",
+            "/Applications/IntelliJ IDEA CE.app",
+        ],
+    ) == [
+        "java",
+        "-Dhello=/Applications/IntelliJ IDEA CE.app/Contents/Info.plist",
+        "-classpath",
+        "/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:/Applications/IntelliJ IDEA CE.app/Contents/plugins/maven/lib/maven3-server-common.jar",
+        "MainClass",
+    ]
+
+
 def test_get_command_python():
     assert px_commandline.get_command("python") == "python"
     assert px_commandline.get_command("/apa/Python") == "Python"
@@ -315,23 +379,23 @@ def test_get_command_sudo():
 
 def test_get_command_sudo_with_space_in_command_name(tmpdir):
     # Create a file name with a space in it
-    spaced_path = tmpdir.join("with space")
+    spaced_path = tmpdir.join("i contain spaces")
     spaced_path.write_binary(b"")
     spaced_name = str(spaced_path)
 
     # Verify splitting of the spaced file name
-    assert px_commandline.get_command("sudo " + spaced_name) == "sudo with space"
+    assert px_commandline.get_command("sudo " + spaced_name) == "sudo i contain spaces"
 
     # Verify splitting with more parameters on the line
     assert (
         px_commandline.get_command("sudo " + spaced_name + " parameter")
-        == "sudo with space"
+        == "sudo i contain spaces"
     )
 
 
 def test_get_command_sudo_with_space_in_path(tmpdir):
     # Create a file name with a space in it
-    spaced_dir = tmpdir.join("with space")
+    spaced_dir = tmpdir.join("i contain spaces")
     os.mkdir(str(spaced_dir))
     spaced_path = spaced_dir + "/runme"
 

tests/px_process_test.py

@@ -284,7 +284,7 @@ def test_get_command_line_array():
 
 def test_get_command_line_array_space_in_binary(tmpdir):
     # Create a file name with a space in it
-    spaced_path = tmpdir.join("with space")
+    spaced_path = tmpdir.join("i contain spaces")
     spaced_path.write_binary(b"")
     spaced_name = str(spaced_path)