[PATCH] D56901: [Support] Fix Windows Command Shell Command line parsing for quoted arguments

[Jonas Toth via Phabricator via llvm-commits]

JonasToth added a comment.

Looking through the clang-tidy eyes as well, my thoughts.



================
Comment at: lib/Support/CommandLine.cpp:447
+    // Pull off the leading and trailing 's.
+    if (Value.size() > 1 && Value[0] == '\'' && Value[Value.size() - 1] == '\'')
+      Value = Value.substr(1, Value.size() - 2);
----------------
I think more specific `StringRef` methods should be used here for readability.

Maybe
```
if (Value.size() >= 2 && Value.startswith('\'') && Value.endswith('\''))
    Value = Value.slice(1, Value.size() - 1); // minus one, because slice takes the index, not the count as substr
```

I would prefer the `>= 2` as it seems clearer to me, that at least 2 character are necessary for this logic. This is highly subjective and I would not insist on anything.


================
Comment at: unittests/Support/CommandLineTest.cpp:916
 }
 
+TEST(CommandLineTest, WindowsCmdShellQuotesArg) {
----------------
changing the file-mode for this file is not necessary, is it?


================
Comment at: unittests/Support/CommandLineTest.cpp:941
+  EXPECT_TRUE(Checks.compare("-*,readability-identifier-naming") == 0);
+}
+
----------------
Could you please add a test with empty input (`-checks=''`, `-checks=`).


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56901/new/

https://reviews.llvm.org/D56901