[PATCH] D13549: Added new options to ClangFormat VSIX package.

Marek Kurdej via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 12 03:15:51 PDT 2015


curdeius added inline comments.

================
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:186
@@ -145,1 +185,3 @@
+            if (!string.IsNullOrEmpty(assumeFilename))
+              process.StartInfo.Arguments += " -assume-filename \"" + assumeFilename + "\"";
             process.StartInfo.CreateNoWindow = true;
----------------
klimek wrote:
> curdeius wrote:
> > klimek wrote:
> > > Don't we need " escaping for assumeFilename? (or do we consider that an error? in which case, would we want to make that an error?)
> > Well, quotes (") are not allowed as a part of a file name on Windows.
> > What we could do is to strip surrounding quotes if the user added them in order not to add them twice.
> Oh, I didn't know that.
> My main concern is what happens when the user (accidentally) adds quotes (for example, unbalanced) and messes up the command line in a way that leads to a super hard to diagnose problem
Hmm, you're right, we should do something with that.
Either:

  - escape all quotes (but then the errors will be silent and user will not know that something bad happens)
  - or to give an error message somehow, e.g. by using a custom TypeConverter for AssumeFilename option that will check for the existence of quotes.


http://reviews.llvm.org/D13549





More information about the cfe-commits mailing list