From 9419ecf1f2b5d7c8445fcf58a1126dd8efadb51d Mon Sep 17 00:00:00 2001 From: Jonah Graham Date: Sun, 2 Feb 2025 12:26:26 -0500 Subject: [PATCH] Improve CommandLineUtil to string methods so they look prettier Initially CommandLineUtil.argumentsToString was used to provide properly quoted string when interacting with GDB. But it is also useful to print a string that can be copied + pasted to a terminal for the user. When doing this the always quote every argument looks less nice, so this change updates the code to only quote argument if needed. Tests have also been added for the quoting. Improves look and feel of changes in #1073 --- .../cdt/utils/CommandLineUtilTest.java | 40 +++++++++ .../eclipse/cdt/utils/CommandLineUtil.java | 87 +++++++++++-------- 2 files changed, 91 insertions(+), 36 deletions(-) diff --git a/core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/utils/CommandLineUtilTest.java b/core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/utils/CommandLineUtilTest.java index 062df03642f..c198af409d3 100644 --- a/core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/utils/CommandLineUtilTest.java +++ b/core/org.eclipse.cdt.core.tests/misc/org/eclipse/cdt/utils/CommandLineUtilTest.java @@ -35,11 +35,24 @@ private String[] parseU(String line) { return CommandLineUtil.argumentsToArray(line); } + private String join(String[] line) { + return CommandLineUtil.argumentsToStringBash(line, false); + } + + private String joinWin(String[] line) { + return CommandLineUtil.argumentsToStringWindowsCreateProcess(line, false); + } + + private String joinU(String[] line) { + return CommandLineUtil.argumentsToString(line, false); + } + public void testArgumentsToArraySimple() { String[] args = parse("A=B C"); assertEquals(2, args.length); assertEquals("A=B", args[0]); assertEquals("C", args[1]); + assertEquals("A=B C", join(args)); } public void testArgumentsToArraySpaces() { @@ -48,6 +61,7 @@ public void testArgumentsToArraySpaces() { assertEquals(2, args.length); assertEquals("A=B", args[0]); assertEquals("C", args[1]); + assertEquals("A=B C", join(args)); } public void testArgumentsToArraySpaces2() { @@ -56,6 +70,7 @@ public void testArgumentsToArraySpaces2() { assertEquals(2, args.length); assertEquals("A=B", args[0]); assertEquals("C", args[1]); + assertEquals("A=B C", join(args)); } public void testArgumentsToArrayDoubleQuotes() { @@ -63,6 +78,7 @@ public void testArgumentsToArrayDoubleQuotes() { String[] args = parse("Arg=\"a b c\""); assertEquals(1, args.length); assertEquals("Arg=a b c", args[0]); + assertEquals("'Arg=a b c'", join(args)); } public void testArgumentsToArrayDoubleQuotes2() { @@ -70,6 +86,7 @@ public void testArgumentsToArrayDoubleQuotes2() { String[] args = parse("Arg=\"\\\"quoted\\\"\""); assertEquals(1, args.length); assertEquals("Arg=\"quoted\"", args[0]); + assertEquals("'Arg=\"quoted\"'", join(args)); } public void testArgumentsToArraySingleQuotes() { @@ -77,6 +94,7 @@ public void testArgumentsToArraySingleQuotes() { String[] args = parse("Arg='\"quoted\"'"); assertEquals(1, args.length); assertEquals("Arg=\"quoted\"", args[0]); + assertEquals("'Arg=\"quoted\"'", join(args)); } public void testArgumentsToArrayQuote() { @@ -84,6 +102,7 @@ public void testArgumentsToArrayQuote() { String[] args = parse("\\\""); assertEquals(1, args.length); assertEquals("\"", args[0]); + assertEquals("'\"'", join(args)); } public void testArgumentsToArrayQuotSpaces() { @@ -91,6 +110,7 @@ public void testArgumentsToArrayQuotSpaces() { String[] args = parse(" \\\""); assertEquals(1, args.length); assertEquals("\"", args[0]); + assertEquals("'\"'", join(args)); } public void testArgumentsToArrayOnlySpaces() { @@ -98,6 +118,7 @@ public void testArgumentsToArrayOnlySpaces() { String[] args = parse("\" \""); assertEquals(1, args.length); assertEquals(" ", args[0]); + assertEquals("' '", join(args)); } public void testArgumentsToArrayJumbledString() { @@ -105,6 +126,7 @@ public void testArgumentsToArrayJumbledString() { String[] args = parse("\"a b\"-c"); assertEquals(1, args.length); assertEquals("a b-c", args[0]); + assertEquals("'a b-c'", join(args)); } public void testArgumentsToArrayJumbledString2() { @@ -113,6 +135,7 @@ public void testArgumentsToArrayJumbledString2() { assertEquals(2, args.length); assertEquals("x", args[0]); assertEquals("a b-c", args[1]); + assertEquals("x 'a b-c'", join(args)); } public void testArgumentsToArrayJumbledSQ() { @@ -121,6 +144,7 @@ public void testArgumentsToArrayJumbledSQ() { assertEquals(2, args.length); assertEquals("x x", args[0]); assertEquals("y", args[1]); + assertEquals("'x x' y", join(args)); } public void testArgumentsToArrayEmptyString() { @@ -128,6 +152,7 @@ public void testArgumentsToArrayEmptyString() { String[] args = parse("\"\""); assertEquals(1, args.length); assertEquals("", args[0]); + assertEquals("''", join(args)); } public void testArgumentsToArrayEmptyString2() { @@ -135,6 +160,7 @@ public void testArgumentsToArrayEmptyString2() { String[] args = parse("''"); assertEquals(1, args.length); assertEquals("", args[0]); + assertEquals("''", join(args)); } public void testArgumentsToArrayEmpty3() { @@ -143,6 +169,7 @@ public void testArgumentsToArrayEmpty3() { assertEquals(2, args.length); assertEquals("", args[0]); assertEquals("a", args[1]); + assertEquals("'' a", join(args)); } public void testArgumentsToArrayQuot1() { @@ -150,6 +177,7 @@ public void testArgumentsToArrayQuot1() { String[] args = parse("'\"'"); assertEquals(1, args.length); assertEquals("\"", args[0]); + assertEquals("'\"'", join(args)); } public void testArgumentsToArrayQuot2() { @@ -157,24 +185,28 @@ public void testArgumentsToArrayQuot2() { String[] args = parse("\"\\\"\""); assertEquals(1, args.length); assertEquals("\"", args[0]); + assertEquals("'\"'", join(args)); } public void testArgumentsToArrayNull() { // [] String[] args = parse(null); assertEquals(0, args.length); + assertEquals("", join(args)); } public void testArgumentsToArrayEmpty() { // [] String[] args = parse(""); assertEquals(0, args.length); + assertEquals("", join(args)); } public void testArgumentsToArrayEmptySpaces() { // [ ] String[] args = parse(" "); assertEquals(0, args.length); + assertEquals("", join(args)); } public void testArgumentsToArrayTabs() { @@ -182,6 +214,7 @@ public void testArgumentsToArrayTabs() { String[] args = parse("a \tb"); assertEquals(2, args.length); assertEquals("a", args[0]); + assertEquals("a b", join(args)); } public void testArgumentsToArrayNL() { @@ -189,6 +222,7 @@ public void testArgumentsToArrayNL() { String[] args = parse("\"a\\nb\""); assertEquals(1, args.length); assertEquals("a\nb", args[0]); + assertEquals("'a\nb'", join(args)); } public void testArgumentsToArraySimpleWin() { @@ -196,24 +230,28 @@ public void testArgumentsToArraySimpleWin() { assertEquals(2, args.length); assertEquals("A=B", args[0]); assertEquals("C", args[1]); + assertEquals("A=B C", joinWin(args)); } public void testArgumentsToArrayWindowsFiles() { String[] args = parseWin("my\\file\\path"); assertEquals(1, args.length); assertEquals("my\\file\\path", args[0]); + assertEquals("my\\file\\path", joinWin(args)); } public void testArgumentsToArrayWindowsSpaces() { String[] args = parseWin("\"my\\file\\path space\""); assertEquals(1, args.length); assertEquals("my\\file\\path space", args[0]); + assertEquals("\"my\\file\\path space\"", joinWin(args)); } public void testArgumentsToArrayWindowsEmpty() { String[] args = parseWin("\"\""); assertEquals(1, args.length); assertEquals("", args[0]); + assertEquals("\"\"", joinWin(args)); } public void testArgumentsToArrayWindowsQuotes() { @@ -221,6 +259,7 @@ public void testArgumentsToArrayWindowsQuotes() { assertEquals(2, args.length); assertEquals("\"a", args[0]); assertEquals("b\"", args[1]); + assertEquals("\"\\\"a\" \"b\\\"\"", joinWin(args)); } public void testArgumentsToArraySimpleUniversal() { @@ -229,5 +268,6 @@ public void testArgumentsToArraySimpleUniversal() { assertEquals("A=B", args[0]); assertEquals("C", args[1]); assertEquals("D", args[2]); + assertEquals("A=B C D", joinU(args)); } } diff --git a/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/CommandLineUtil.java b/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/CommandLineUtil.java index 5d91d7faa10..4b8b903cf46 100644 --- a/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/CommandLineUtil.java +++ b/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/CommandLineUtil.java @@ -346,20 +346,24 @@ public static String argumentsToStringBash(String[] args, boolean encodeNewline) builder.append(' '); } - builder.append('\''); - for (int j = 0; j < arg.length(); j++) { - char c = arg.charAt(j); - if (c == '\'') { - builder.append("'\"'\"'"); //$NON-NLS-1$ - } else if (c == '\r' && encodeNewline) { - builder.append("'$'\\r''"); //$NON-NLS-1$ - } else if (c == '\n' && encodeNewline) { - builder.append("'$'\\n''"); //$NON-NLS-1$ - } else { - builder.append(c); + if (needsQuoting(arg)) { + builder.append('\''); + for (int j = 0; j < arg.length(); j++) { + char c = arg.charAt(j); + if (c == '\'') { + builder.append("'\"'\"'"); //$NON-NLS-1$ + } else if (c == '\r' && encodeNewline) { + builder.append("'$'\\r''"); //$NON-NLS-1$ + } else if (c == '\n' && encodeNewline) { + builder.append("'$'\\n''"); //$NON-NLS-1$ + } else { + builder.append(c); + } } + builder.append('\''); + } else { + builder.append(arg); } - builder.append('\''); } return builder.toString(); @@ -386,39 +390,50 @@ public static String argumentsToStringWindowsCreateProcess(String[] args, boolea builder.append(' '); } - builder.append('"'); - for (int j = 0; j < arg.length(); j++) { - /* - * backslashes are special if and only if they are followed by a - * double-quote (") therefore doubling them depends on what is - * next - */ - int numBackslashes = 0; - for (; j < arg.length() && arg.charAt(j) == '\\'; j++) { - numBackslashes++; - } - if (j == arg.length()) { - appendNBackslashes(builder, numBackslashes * 2); - } else if (arg.charAt(j) == '"') { - appendNBackslashes(builder, numBackslashes * 2); - builder.append('"'); - } else if ((arg.charAt(j) == '\n' || arg.charAt(j) == '\r') && encodeNewline) { - builder.append(' '); - } else { + if (needsQuoting(arg)) { + builder.append('"'); + for (int j = 0; j < arg.length(); j++) { /* - * this really is numBackslashes (no missing * 2), that is - * because next character is not a double-quote (") + * backslashes are special if and only if they are followed by a + * double-quote (") therefore doubling them depends on what is + * next */ - appendNBackslashes(builder, numBackslashes); - builder.append(arg.charAt(j)); + int numBackslashes = 0; + for (; j < arg.length() && arg.charAt(j) == '\\'; j++) { + numBackslashes++; + } + if (j == arg.length()) { + appendNBackslashes(builder, numBackslashes * 2); + } else if (arg.charAt(j) == '"') { + appendNBackslashes(builder, numBackslashes * 2); + builder.append('\\'); + builder.append('"'); + } else if ((arg.charAt(j) == '\n' || arg.charAt(j) == '\r') && encodeNewline) { + builder.append(' '); + } else { + /* + * this really is numBackslashes (no missing * 2), that is + * because next character is not a double-quote (") + */ + appendNBackslashes(builder, numBackslashes); + builder.append(arg.charAt(j)); + } } + builder.append('"'); + } else { + builder.append(arg); } - builder.append('"'); } return builder.toString(); } + private static boolean needsQuoting(String arg) { + boolean needsQuoting = arg.isBlank() || arg.chars().mapToObj(i -> (char) i) + .anyMatch(c1 -> Character.isWhitespace(c1) || c1 == '"' || c1 == '\''); + return needsQuoting; + } + private static void appendNBackslashes(StringBuilder builder, int numBackslashes) { for (int i = 0; i < numBackslashes; i++) { builder.append('\\');