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

[MyDeveloperDay via Phabricator via llvm-commits]

MyDeveloperDay marked 6 inline comments as done.
MyDeveloperDay added inline comments.


================
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);
----------------
JonasToth wrote:
> 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.
sounds good, when I rewrite it I'll do it that way. (or something like it)


================
Comment at: unittests/Support/CommandLineTest.cpp:941
+  EXPECT_TRUE(Checks.compare("-*,readability-identifier-naming") == 0);
+}
+
----------------
JonasToth wrote:
> Could you please add a test with empty input (`-checks=''`, `-checks=`).
let me do this when I do it up in clang-tidy.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56901