Also, if you want to update the patch, you don't need to abandon the revision - if you abandon the revision there is not much more you can do with it... (and I cannot patch it in and see what you actually are trying to fix).<br><div><br></div><div>The general idea behind the unescaping is that we want to be able to put shell commands "as if you wanted to pass them to shell" into the "command" entry...</div><br><div class="gmail_quote">On Mon Nov 24 2014 at 11:03:24 AM Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote">On Mon Nov 24 2014 at 10:57:12 AM Daniel Cheng <<a href="mailto:dcheng@google.com" target="_blank">dcheng@google.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I abandoned the previous diff, because I was flailing about the interface and had no idea what I was doing.<br>
<br>
As for the fix, the JSON compilation database spec (<a href="http://clang.llvm.org/docs/JSONCompilationDatabase.html" target="_blank">http://clang.llvm.org/docs/<u></u>JS<u></u>ONCompilationDatabase.html</a>) says:<br>
> Parameters use shell quoting and shell escaping of quotes.<br>
<br>
The problem is that `skipEscapeCharacter()` always skips over a backslash character, instead of the behavior described in the spec (only shell escaping of quotes). Ninja and CMake expect that shell escaping is only applied to quotes, and output compile database commands like:<br>
```"clang-cl.exe path\\to\\input.cc"```<br>
<br>
After JSON unescaping, the command looks like:<br>
```clang-cl.exe path\to\input.cc```<br>
<br>
But because the argument parser always skips backslashes, `JSONCompilationDatabase::<u></u>getC<u></u>ompileCommands` ends up returning:<br>
```clang-cl.exe pathtoinput.cc```<br>
<br>
Which obviously doesn't work. One alternative is to decide that all normal shell escapes apply and change the spec. This has one big disadvantage: the JSON decoder already turns escape sequences into their special character (e.g. `\n` to an actual newline character). Why does the argument unescaper need to do the same thing?<br>
<br>
Finally, there were no tests for the command argument unescaping behavior, so I modified `JSONCompilationDatabase.<u></u>GetAl</blockquote><div><br></div></div><div class="gmail_quote"><div>What about these?</div><div><a href="http://reviews.llvm.org/diffusion/L/browse/cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp;222646$294" target="_blank">http://reviews.llvm.org/diffusion/L/browse/cfe/trunk/unittests/Tooling/CompilationDatabaseTest.cpp;222646$294</a><br></div></div><div class="gmail_quote"><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><u></u>lCompileCommands` to cover several argument parser behaviors:<br>
- Double quotes allow spaces to be included in a single argument.<br>
- Escaped quotes are skipped over inside a double quoted argument.<br>
- Backslashes are included in the unescaped argument if they weren't escaping a quote.<br>
- Backslashes are allowed as the last character of a single argument.<br>
<br>
<a href="http://reviews.llvm.org/D6376" target="_blank">http://reviews.llvm.org/D6376</a><br>
<br>
<br>
</blockquote></div></blockquote></div>