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
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
>
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
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.
> 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
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.
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
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.
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
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
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);
+
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
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
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
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
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)));
}
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
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 + "\"";
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)));
}
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 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 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/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
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
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 + "\"";
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
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
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
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
30 matches
Mail list logo
