On Mon, Oct 19, 2015 at 7:45 PM, Hans Wennborg via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> hans added a comment.
>
> This is now part of the latest snapshot at http://llvm.org/builds/
> Seems to work :-)
>
> I had to dig around a bit to figure out where these settings are actually
> exp
hans added a comment.
This is now part of the latest snapshot at http://llvm.org/builds/
Seems to work :-)
I had to dig around a bit to figure out where these settings are actually
exposed. Should we mention that in the documentation somewhere? Actually, do we
even have any documentation for th
zturner added a comment.
Oh I see. Makes sense then.
http://reviews.llvm.org/D13549
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
curdeius added a comment.
> Where is the code in the CL that handles extracting that value from a JSON
> string? Because it looks like you're just building an array list of the
> trivial non-JSON style names, so why couldn't that be an enum? I don't know
> mucha bout clang-format, but looking
zturner added a comment.
In http://reviews.llvm.org/D13549#268701, @curdeius wrote:
> Hi Zachary, just to answer your comments. I have done it on purpose not to
> use enum, because clang-format style can be actually a JSON string, e.g.
> `{BasedOnStyle: "LLVM", IndentWidth: 4}`, so it wouldn't
curdeius added a comment.
Hi Zachary, just to answer your comments. I have done it on purpose not to use
enum, because clang-format style can be actually a JSON string, e.g.
`{BasedOnStyle: "LLVM", IndentWidth: 4}`, so it wouldn't translate into an enum
(to my knowledge at least). Besides, I wo
zturner added a comment.
No obvious problems, mostly just style issues. Feel free to consider them
optional.
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:38
@@ +37,3 @@
+private bool sortIncludes = false;
+private string style = "file";
+
rnk added a comment.
In http://reviews.llvm.org/D13549#267790, @klimek wrote:
> We're waiting for Reid to find somebody who is good at reviewing this in
> detail.
> Sorry it takes a while, so far we don't have enough trusted Windows experts
> in the community :(
Oops, I didn't know that. Zac
djasper added a comment.
While we are waiting, submitted the changes to ClangFormat.cpp in r250440.
Thank you!
http://reviews.llvm.org/D13549
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe
klimek added a comment.
We're waiting for Reid to find somebody who is good at reviewing this in detail.
Sorry it takes a while, so far we don't have enough trusted Windows experts in
the community :(
http://reviews.llvm.org/D13549
___
cfe-commits
curdeius added a comment.
Ping?
http://reviews.llvm.org/D13549
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
curdeius marked 7 inline comments as done.
curdeius added a comment.
Applied comments and done some minor clean up.
http://reviews.llvm.org/D13549
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo
curdeius updated this revision to Diff 37253.
curdeius added a comment.
Applied Aaron's comments. Removed unused using.
http://reviews.llvm.org/D13549
Files:
tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
tools/clang-fo
aaron.ballman added inline comments.
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:82
@@ -42,1 +81,3 @@
+ " - Predefined styles ('LLVM', 'Google', 'Chromium',
'Mozilla', 'WebKit').\n" +
+ " - 'file' to search for a Y
curdeius added inline comments.
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:57
@@ +56,3 @@
+{
+StandardValuesCollection svc = new
StandardValuesCollection(values);
+return svc;
aaron.ballman wrot
aaron.ballman added a comment.
Generally looks good to me, with a few small nits.
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:57
@@ +56,3 @@
+{
+StandardValuesCollection svc = new
StandardValuesCollection(values);
+
klimek added reviewers: aaron.ballman, rnk.
klimek added a comment.
+aaron, as token Windows Wizard
+rnk, as I was told you might know people who are in a good position to review
MS specific stuff (perhaps engineers from MS might be able to help here? :)
http://reviews.llvm.org/D13549
_
curdeius updated this revision to Diff 37115.
curdeius added a comment.
Add option converters for filenames (prohibiting quotes) and styles (giving a
list of predefined styles).
http://reviews.llvm.org/D13549
Files:
tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
tools/clang-format
curdeius marked 5 inline comments as done.
curdeius added a comment.
Assume-Filename option will now give an error when there are quotes in the
filename.
http://reviews.llvm.org/D13549
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://
klimek 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 + "\"";
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 + "\"";
klimek 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 + "\"";
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 + "\"";
klimek 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 + "\"";
curdeius marked 3 inline comments as done.
curdeius added a comment.
Applied suggestions from comments.
http://reviews.llvm.org/D13549
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
curdeius updated this revision to Diff 37087.
curdeius marked 2 inline comments as done.
curdeius added a comment.
Add FIXME comment.
http://reviews.llvm.org/D13549
Files:
tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
curdeius updated this revision to Diff 37086.
curdeius added a comment.
Escape only '<' and '&'.
Remove unrelated changes from the revision.
http://reviews.llvm.org/D13549
Files:
tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
tools/clang-format-vs/ClangFormat/Properties/AssemblyInf
klimek added inline comments.
Comment at: tools/clang-format/ClangFormat.cpp:227
@@ -213,1 +226,3 @@
+ llvm::outs() << """;
+ break;
default:
curdeius wrote:
> klimek wrote:
> > For XML, we should only need "<" and "&". Everything else is just impor
curdeius added inline comments.
Comment at: lib/Driver/Tools.cpp:2356
@@ -2355,3 +2355,3 @@
CmdArgs.push_back(
-Args.MakeArgString("-dwarf-version=" + llvm::itostr(DwarfVersion)));
+Args.MakeArgString("-dwarf-version=" + Twine(DwarfVersion)));
}
-
klimek added inline comments.
Comment at: lib/Driver/Tools.cpp:2356
@@ -2355,3 +2355,3 @@
CmdArgs.push_back(
-Args.MakeArgString("-dwarf-version=" + llvm::itostr(DwarfVersion)));
+Args.MakeArgString("-dwarf-version=" + Twine(DwarfVersion)));
}
---
curdeius updated this revision to Diff 37081.
curdeius added a comment.
Fix description formatting.
http://reviews.llvm.org/D13549
Files:
lib/Driver/Tools.cpp
tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
tools/clang
curdeius updated this revision to Diff 36849.
curdeius added a comment.
Escape XML-reserved characters.
http://reviews.llvm.org/D13549
Files:
tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
tools/clang-format/ClangFormat
djasper added a comment.
Please always add cfe-commits as "subscriber" so that the email also goes to
the list.
Repository:
rL LLVM
http://reviews.llvm.org/D13549
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-
33 matches
Mail list logo
