From cb49bce964ee3318f7f721a1fe4510bdba77ae30 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Wed, 6 Mar 2024 18:34:56 +0100 Subject: [PATCH 1/5] Test splitting MS Edge command line Fails. --- tests/px_commandline_test.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/px_commandline_test.py b/tests/px_commandline_test.py index dfa34bc..dc40671 100644 --- a/tests/px_commandline_test.py +++ b/tests/px_commandline_test.py @@ -134,6 +134,37 @@ def test_to_array_spaced3(): ] +def test_to_array_ms_edge(): + complete = "/".join( + [ + "/Applications", + "Microsoft Edge.app", + "Contents", + "Frameworks", + "Microsoft Edge Framework.framework", + "Versions", + "122.0.2365.63", + "Helpers", + "Microsoft Edge Helper (GPU).app", + "Contents", + "MacOS", + "Microsoft Edge Helper (GPU)", + ] + ) + exists = [] + partial = complete + while True: + exists.append(partial) + partial = os.path.dirname(partial) + if partial == "/": + break + + assert px_commandline.to_array( + complete + " --type=gpu-process", + exists=lambda s: s in exists, + ) == [complete] + ["--type=gpu-process"] + + def test_get_command_python(): assert px_commandline.get_command("python") == "python" assert px_commandline.get_command("/apa/Python") == "Python" From 791a0de710484ab13ffa30425583df6d3e885e7c Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Wed, 6 Mar 2024 18:38:03 +0100 Subject: [PATCH 2/5] Add another should_coalesce test Failing. Fix this first, then the other test probably fixes itself. --- tests/px_commandline_test.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/px_commandline_test.py b/tests/px_commandline_test.py index dc40671..aaccf08 100644 --- a/tests/px_commandline_test.py +++ b/tests/px_commandline_test.py @@ -34,6 +34,11 @@ def exists(s): exists=exists, ) + 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"], + ) + def test_coalesce_count(): def exists(s): From 0a4baced1e05336841fa8f7c436cfe9cb20e8f6e Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Sun, 10 Mar 2024 15:15:20 +0100 Subject: [PATCH 3/5] Move : responsibility into should_coalesce() --- px/px_commandline.py | 7 +------ tests/px_commandline_test.py | 5 +++++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/px/px_commandline.py b/px/px_commandline.py index 0e196b1..ffa2696 100644 --- a/px/px_commandline.py +++ b/px/px_commandline.py @@ -100,9 +100,8 @@ def coalesce_count( # 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) + should_coalesce_ = should_coalesce(parts[0:coalesce_count], exists) if should_coalesce_ is None: # Undecided, keep looking @@ -114,10 +113,6 @@ def coalesce_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 diff --git a/tests/px_commandline_test.py b/tests/px_commandline_test.py index aaccf08..e94e18e 100644 --- a/tests/px_commandline_test.py +++ b/tests/px_commandline_test.py @@ -39,6 +39,11 @@ def exists(s): 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"], + ) + def test_coalesce_count(): def exists(s): From cb37a0a700b6a17b4532e4dca2e0ea4c1053e7d2 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Sun, 10 Mar 2024 15:54:33 +0100 Subject: [PATCH 4/5] Pass some tests --- px/px_commandline.py | 64 +++++++++++++++--------------------- tests/px_commandline_test.py | 50 ++++++++++++++++++---------- 2 files changed, 59 insertions(+), 55 deletions(-) diff --git a/px/px_commandline.py b/px/px_commandline.py index ffa2696..b74fcb1 100644 --- a/px/px_commandline.py +++ b/px/px_commandline.py @@ -25,18 +25,14 @@ def should_coalesce( """ 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. + path, or a : separated series of file paths. 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. + * False: Do not coalesce, leave these parts alone + * None: Do not coalesce, but if you add more parts then that might work """ - if parts[0].endswith("/"): - # "xxx/ yyy" would make no sense coalesced - return False - if parts[-1].startswith("-"): # "xxx -yyy" would make no sense coalesced, that - likely means what # comes after is a command line switch @@ -46,50 +42,44 @@ def should_coalesce( # "xxx /yyy" would make no sense coalesced return False - # Find the last possible starting point of an absolute path in part1 + 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 parts[0].startswith("/"): + + if coalesced.startswith("/"): # /x/y/z path_start_index = 0 - if (first_equals_slash := parts[0].find("=/")) >= 0: + if (first_equals_slash := coalesced.find("=/")) >= 0: # -Dhello=/x/y/z path_start_index = first_equals_slash + 1 - if (last_colon_slash := parts[0].rfind(":/")) >= 0: + 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: - # Part 1 does not contain the start of any path, do not coalesce + # Absolute path not found, do not coalesce return False - path_end_index_exclusive = len(parts[-1]) - if (first_colon := parts[-1].find(":")) >= 0: - path_end_index_exclusive = first_colon - if (first_slash := parts[-1].find("/")) >= 0: - if first_slash < path_end_index_exclusive: - path_end_index_exclusive = first_slash - - middle = " " - if len(parts) > 2: - middle = " " + " ".join(parts[1:-1]) + " " - - candidate_path = ( - parts[0][path_start_index:] + middle + parts[-1][:path_end_index_exclusive] - ) + path_end_index_exclusive = len(coalesced) + if (last_colon := coalesced.rfind(":")) >= 0: + if last_colon > path_start_index: + path_end_index_exclusive = last_colon - _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 + 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: - # No hit, and we got something with slashes in it, so this is - # definitely a no. - return False + # Candidate exists, but input continues, keep looking + return None + else: + # Candidate does not exist, but maybe it's incomplete, keep looking + return None def coalesce_count( diff --git a/tests/px_commandline_test.py b/tests/px_commandline_test.py index e94e18e..52dfcd3 100644 --- a/tests/px_commandline_test.py +++ b/tests/px_commandline_test.py @@ -14,24 +14,33 @@ def exists(s): ["java", "-Dhello=/Applications/IntelliJ"], exists=exists ) - assert px_commandline.should_coalesce( - ["-Dhello=/Applications/IntelliJ", "IDEA.app/Contents"], exists=exists + 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, + 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.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, + 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, + ) + is None # Potentially incomplete ) assert px_commandline.should_coalesce( @@ -80,6 +89,7 @@ def test_to_array_spaced1(): in [ "/Applications", "/Applications/IntelliJ IDEA.app", + "/Applications/IntelliJ IDEA.app/Contents", ], ) == ["java", "-Dhello=/Applications/IntelliJ IDEA.app/Contents"] @@ -100,8 +110,10 @@ def test_to_array_spaced2(): ), exists=lambda s: s in [ - "/Applications", - "/Applications/IntelliJ IDEA.app", + "/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", + "/Applications/IntelliJ IDEA.app/Contents/plugins/maven/lib/maven3-server-common.jar", ], ) == [ "java", @@ -132,8 +144,10 @@ def test_to_array_spaced3(): ), exists=lambda s: s in [ - "/Applications", - "/Applications/IntelliJ IDEA CE.app", + "/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", + "/Applications/IntelliJ IDEA CE.app/Contents/plugins/maven/lib/maven3-server-common.jar", ], ) == [ "java", From 3282f717c2acc6ffb10ded62638bb08767a1ef90 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Sun, 10 Mar 2024 23:35:04 +0100 Subject: [PATCH 5/5] Extract candidate identification --- px/px_commandline.py | 95 +++++++++++++++++------------------- tests/px_commandline_test.py | 57 +++++----------------- 2 files changed, 59 insertions(+), 93 deletions(-) diff --git a/px/px_commandline.py b/px/px_commandline.py index b74fcb1..74b969f 100644 --- a/px/px_commandline.py +++ b/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( diff --git a/tests/px_commandline_test.py b/tests/px_commandline_test.py index 52dfcd3..998231d 100644 --- a/tests/px_commandline_test.py +++ b/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",