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

2015-10-19 Thread Hans Wennborg via cfe-commits
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 this plugin?


http://reviews.llvm.org/D13549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2015-10-19 Thread Ismail Donmez via cfe-commits
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
> exposed. Should we mention that in the documentation somewhere? Actually,
> do we even have any documentation for this plugin?
>
>
Options are exposed in Tools->Options->LLVM/Clang->ClangFormat it seems
(which is very nice btw!)

ismail
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2015-10-16 Thread Marek Kurdej via cfe-commits
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 would like to have the possibility 
**not** to add a new enum value if there is a new style in clang-format.
Please correct me if I'm wrong about enums.



Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:115
@@ +114,3 @@
+// Check if string contains quotes. On Windows, file names 
cannot contain quotes.
+// We do not accept them however to avoid hard-to-debug 
problems.
+// A quote in user input would end the parameter quote and so 
break the command invocation.

zturner wrote:
> Wouldn't it be better to just return `false` from `CanConvertFrom` if the 
> filename contains quotes?  Then you can assert in this function that it does 
> not contain quotes?
Well, in `CanConvertFrom`, the `sourceType` parameter is a `Type` and not the 
value given by the user, so no, we cannot do this.


http://reviews.llvm.org/D13549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2015-10-16 Thread Zachary Turner via cfe-commits
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 translate into an 
> enum (to my knowledge at least). Besides, I would like to have the 
> possibility **not** to add a new enum value if there is a new style in 
> clang-format.
>  Please correct me if I'm wrong about enums.


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 at the comments it seems like the JSON 
only comes into play when you're reading a .clang-format file, and in that case 
the value of the enum would be `Style.File` (until you finish reading that, at 
which case you set it to `Style.LLVM` etc).

I'm also not sure what advantage you get by not having to add an enum if there 
is a new style?  You have to change the code either way, and by not using an 
enum you are likely to miss some of the places in the code that manipulate or 
process the style since you're bypassing the type system.


http://reviews.llvm.org/D13549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2015-10-16 Thread Marek Kurdej via 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 at the comments it seems like the JSON 
> only comes into play when you're reading a .clang-format file, and in that 
> case the value of the enum would be `Style.File` (until you finish reading 
> that, at which case you set it to `Style.LLVM` etc).


The Style option is actually passed to clang-format -style parameter and it's 
clang-format that parses it. It can be a predefined type (one of: 'none', 
'file', 'LLVM', etc.) or a JSON (or JSON-like?) string.

> I'm also not sure what advantage you get by not having to add an enum if 
> there is a new style?  You have to change the code either way, and by not 
> using an enum you are likely to miss some of the places in the code that 
> manipulate or process the style since you're bypassing the type system.


I wanted to say that it would be better if the user didn't have to wait until 
the VSIX plugins gets updated after a new style is added to clang-format.


http://reviews.llvm.org/D13549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2015-10-16 Thread Zachary Turner via cfe-commits
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


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

2015-10-15 Thread Marek Kurdej via 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


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

2015-10-15 Thread Zachary Turner via 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";
+

How about an enum instead?  

private enum Style {
File,
Chromium,
Google,
LLVM,
Mozilla,
Webkit
}
private Style style;


Then change the `StyleConverter` to just convert the enum value to its 
corresponding string representation?


Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:42-47
@@ +41,8 @@
+{
+protected ArrayList values;
+public StyleConverter()
+{
+// Initializes the standard values list with defaults.
+values = new ArrayList(new string[] { "file", "Chromium", 
"Google", "LLVM", "Mozilla", "WebKit" });
+}
+

This would all go away


Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:56
@@ +55,3 @@
+{
+return new StandardValuesCollection(values);
+}

This becomes `return new 
StandardValuesCollection(Enum.GetValues(typeof(Style)))`


Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:61-64
@@ +60,6 @@
+{
+if (sourceType == typeof(string))
+return true;
+
+return base.CanConvertFrom(context, sourceType);
+}

This becomes `sourceType == typeof(Style) || base.CanConvertFrom(context, 
sourceType)`


Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:69-73
@@ +68,7 @@
+{
+string s = value as string;
+if (s == null)
+return base.ConvertFrom(context, culture, value);
+
+return value;
+}

This becomes

Style s = value as Style;
if (value == null)
return base.ConvertFrom(context, culture, value);
return value.ToString();

(I think, I can't tell what the source and destination type are supposed to be 
here)


Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:92
@@ -51,2 +91,3 @@
+[TypeConverter(typeof(StyleConverter))]
 public string Style
 {

Now you can return an enum here instead of a string


Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:115
@@ +114,3 @@
+// Check if string contains quotes. On Windows, file names 
cannot contain quotes.
+// We do not accept them however to avoid hard-to-debug 
problems.
+// A quote in user input would end the parameter quote and so 
break the command invocation.

Wouldn't it be better to just return `false` from `CanConvertFrom` if the 
filename contains quotes?  Then you can assert in this function that it does 
not contain quotes?


Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:136-143
@@ +135,10 @@
+
+public sealed class FallbackStyleConverter : StyleConverter
+{
+public FallbackStyleConverter()
+{
+// Add "none" to the list of styles.
+values.Insert(0, "none");
+}
+}
+

Same comment as before, but use the enum.s  I guess you would need to make 
`None` one of the actual style enum values, but remove it before returning the 
list in the other StyleConverter.  


Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:331-347
@@ -213,1 +330,19 @@
 
+private string GetAssumeFilename()
+{
+var page = (OptionPageGrid)GetDialogPage(typeof(OptionPageGrid));
+return page.AssumeFilename;
+}
+
+private string GetFallbackStyle()
+{
+var page = (OptionPageGrid)GetDialogPage(typeof(OptionPageGrid));
+return page.FallbackStyle;
+}
+
+private bool GetSortIncludes()
+{
+var page = (OptionPageGrid)GetDialogPage(typeof(OptionPageGrid));
+return page.SortIncludes;
+}
+

Maybe you could use properties instead of functions.


http://reviews.llvm.org/D13549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2015-10-15 Thread Reid Kleckner via cfe-commits
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. Zach knows C#, but I don't think any of us are VSIX 
experts.


http://reviews.llvm.org/D13549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2015-10-15 Thread Daniel Jasper via cfe-commits
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-commits


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

2015-10-13 Thread Aaron Ballman via cfe-commits
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 YAML .clang-format or 
_clang-format\n" +
  "configuration file.\n" +

curdeius wrote:
> aaron.ballman wrote:
> > Why is 'file' now lowercased?
> I wanted to be consistent with clang-format --help. Besides, lowercase styles 
> (file, none) are somewhat special and it's nice to have them stand out, IMO.
That seems reasonable to me. Just making sure it wasn't a drive-by unintended 
change, or had some weird semantic meaning in MSVC.


http://reviews.llvm.org/D13549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2015-10-13 Thread Aaron Ballman via cfe-commits
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);
+return svc;

Can we just return directly here instead of storing in a local?


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 YAML .clang-format or 
_clang-format\n" +
  "configuration file.\n" +

Why is 'file' now lowercased?


Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:116
@@ +115,3 @@
+// Check if string contains quotes.
+if (s.IndexOf('\"') != -1)
+throw new NotSupportedException("Filename cannot contain 
quotes");

I see now why this is being used, but it took a lot of context from review 
comments to understand. Can you please put in a more descriptive comment about 
why we're checking for quotes, despite them being invalid filename characters 
on Windows?


http://reviews.llvm.org/D13549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2015-10-13 Thread Marek Kurdej via cfe-commits
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 wrote:
> Can we just return directly here instead of storing in a local?
Hmm, yes, we can.


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 YAML .clang-format or 
_clang-format\n" +
  "configuration file.\n" +

aaron.ballman wrote:
> Why is 'file' now lowercased?
I wanted to be consistent with clang-format --help. Besides, lowercase styles 
(file, none) are somewhat special and it's nice to have them stand out, IMO.


Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:116
@@ +115,3 @@
+// Check if string contains quotes.
+if (s.IndexOf('\"') != -1)
+throw new NotSupportedException("Filename cannot contain 
quotes");

aaron.ballman wrote:
> I see now why this is being used, but it took a lot of context from review 
> comments to understand. Can you please put in a more descriptive comment 
> about why we're checking for quotes, despite them being invalid filename 
> characters on Windows?
OK, will do.


http://reviews.llvm.org/D13549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2015-10-13 Thread Marek Kurdej via cfe-commits
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-format/ClangFormat.cpp

Index: tools/clang-format/ClangFormat.cpp
===
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -199,17 +199,25 @@
 }
 
 static void outputReplacementXML(StringRef Text) {
+  // FIXME: When we sort includes, we need to make sure the stream is correct
+  // utf-8.
   size_t From = 0;
   size_t Index;
-  while ((Index = Text.find_first_of("\n\r", From)) != StringRef::npos) {
+  while ((Index = Text.find_first_of("\n\r<&", From)) != StringRef::npos) {
 llvm::outs() << Text.substr(From, Index - From);
 switch (Text[Index]) {
 case '\n':
   llvm::outs() << "";
   break;
 case '\r':
   llvm::outs() << "";
   break;
+case '<':
+  llvm::outs() << "";
+  break;
+case '&':
+  llvm::outs() << "";
+  break;
 default:
   llvm_unreachable("Unexpected character encountered!");
 }
Index: tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
===
--- tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
+++ tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
@@ -29,5 +29,5 @@
 // You can specify all the values or you can default the Revision and Build Numbers 
 // by using the '*' as shown below:
 
-[assembly: AssemblyVersion("1.0.0.0")]
-[assembly: AssemblyFileVersion("1.0.0.0")]
+[assembly: AssemblyVersion("1.1.0.0")]
+[assembly: AssemblyFileVersion("1.1.0.0")]
Index: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
===
--- tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
+++ tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
@@ -19,10 +19,10 @@
 using Microsoft.VisualStudio.Text.Editor;
 using Microsoft.VisualStudio.TextManager.Interop;
 using System;
+using System.Collections;
 using System.ComponentModel;
 using System.ComponentModel.Design;
 using System.IO;
-using System.Reflection;
 using System.Runtime.InteropServices;
 using System.Xml.Linq;
 
@@ -32,13 +32,53 @@
 [CLSCompliant(false), ComVisible(true)]
 public class OptionPageGrid : DialogPage
 {
-private string style = "File";
+private string assumeFilename = "";
+private string fallbackStyle = "LLVM";
+private bool sortIncludes = false;
+private string style = "file";
+
+public class StyleConverter : TypeConverter
+{
+protected ArrayList values;
+public StyleConverter()
+{
+// Initializes the standard values list with defaults.
+values = new ArrayList(new string[] { "file", "Chromium", "Google", "LLVM", "Mozilla", "WebKit" });
+}
+
+public override bool GetStandardValuesSupported(ITypeDescriptorContext context)
+{
+return true;
+}
+
+public override StandardValuesCollection GetStandardValues(ITypeDescriptorContext context)
+{
+return new StandardValuesCollection(values);
+}
+
+public override bool CanConvertFrom(ITypeDescriptorContext context, Type sourceType)
+{
+if (sourceType == typeof(string))
+return true;
+
+return base.CanConvertFrom(context, sourceType);
+}
+
+public override object ConvertFrom(ITypeDescriptorContext context, System.Globalization.CultureInfo culture, object value)
+{
+string s = value as string;
+if (s == null)
+return base.ConvertFrom(context, culture, value);
+
+return value;
+}
+}
 
 [Category("LLVM/Clang")]
 [DisplayName("Style")]
 [Description("Coding style, currently supports:\n" +
- "  - Predefined styles ('LLVM', 'Google', 'Chromium', 'Mozilla').\n" +
- "  - 'File' to search for a YAML .clang-format or _clang-format\n" +
+ "  - Predefined styles ('LLVM', 'Google', 'Chromium', 'Mozilla', 'WebKit').\n" +
+ "  - 'file' to search for a YAML .clang-format or _clang-format\n" +
  "configuration file.\n" +
  "  - A YAML configuration snippet.\n\n" +
  "'File':\n" +
@@ -48,11 +88,81 @@
  "  The content of a .clang-format configuration file, as string.\n" +
  "  Example: '{BasedOnStyle: \"LLVM\", IndentWidth: 8}'\n\n" +
  "See 

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

2015-10-13 Thread Marek Kurdej via 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/cfe-commits


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

2015-10-12 Thread Marek Kurdej via cfe-commits
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-format/ClangFormat.cpp

Index: tools/clang-format/ClangFormat.cpp
===
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -201,15 +201,30 @@
 static void outputReplacementXML(StringRef Text) {
   size_t From = 0;
   size_t Index;
-  while ((Index = Text.find_first_of("\n\r", From)) != StringRef::npos) {
+  while ((Index = Text.find_first_of("\n\r<>&\"'", From)) != StringRef::npos) {
 llvm::outs() << Text.substr(From, Index - From);
 switch (Text[Index]) {
 case '\n':
   llvm::outs() << "";
   break;
 case '\r':
   llvm::outs() << "";
   break;
+case '<':
+  llvm::outs() << "";
+  break;
+case '>':
+  llvm::outs() << "";
+  break;
+case '&':
+  llvm::outs() << "";
+  break;
+case '\'':
+  llvm::outs() << "";
+  break;
+case '"':
+  llvm::outs() << "";
+  break;
 default:
   llvm_unreachable("Unexpected character encountered!");
 }
Index: tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
===
--- tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
+++ tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
@@ -29,5 +29,5 @@
 // You can specify all the values or you can default the Revision and Build Numbers 
 // by using the '*' as shown below:
 
-[assembly: AssemblyVersion("1.0.0.0")]
-[assembly: AssemblyFileVersion("1.0.0.0")]
+[assembly: AssemblyVersion("1.1.0.0")]
+[assembly: AssemblyFileVersion("1.1.0.0")]
Index: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
===
--- tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
+++ tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
@@ -32,13 +32,16 @@
 [CLSCompliant(false), ComVisible(true)]
 public class OptionPageGrid : DialogPage
 {
-private string style = "File";
+private string assumeFilename = "";
+private string fallbackStyle = "LLVM";
+private bool sortIncludes = false;
+private string style = "file";
 
 [Category("LLVM/Clang")]
 [DisplayName("Style")]
 [Description("Coding style, currently supports:\n" +
- "  - Predefined styles ('LLVM', 'Google', 'Chromium', 'Mozilla').\n" +
- "  - 'File' to search for a YAML .clang-format or _clang-format\n" +
+ "  - Predefined styles ('LLVM', 'Google', 'Chromium', 'Mozilla', 'WebKit').\n" +
+ "  - 'file' to search for a YAML .clang-format or _clang-format\n" +
  "configuration file.\n" +
  "  - A YAML configuration snippet.\n\n" +
  "'File':\n" +
@@ -53,6 +56,38 @@
 get { return style; }
 set { style = value; }
 }
+
+[Category("LLVM/Clang")]
+[DisplayName("Assume Filename")]
+[Description("When reading from stdin, clang-format assumes this " +
+ "filename to look for a style config file (with 'file' style) " +
+ "and to determine the language.")]
+public string AssumeFilename
+{
+get { return assumeFilename; }
+set { assumeFilename = value; }
+}
+
+[Category("LLVM/Clang")]
+[DisplayName("Fallback Style")]
+[Description("The name of the predefined style used as a fallback in case clang-format " +
+ "is invoked with 'file' style, but can not find the configuration file.\n" +
+ "Use 'none' fallback style to skip formatting.")]
+public string FallbackStyle
+{
+get { return fallbackStyle; }
+set { fallbackStyle = value; }
+}
+
+[Category("LLVM/Clang")]
+[DisplayName("Sort includes")]
+[Description("Sort touched include lines.\n\n" +
+ "See also: http://clang.llvm.org/docs/ClangFormat.html.;)]
+public bool SortIncludes
+{
+get { return sortIncludes; }
+set { sortIncludes = value; }
+}
 }
 
 [PackageRegistration(UseManagedResourcesOnly = true)]
@@ -138,10 +173,17 @@
 // Poor man's escaping - this will not work when quotes are already escaped
 // in the input (but we don't need more).
 string style = GetStyle().Replace("\"", "\\\"");
+string fallbackStyle = GetFallbackStyle().Replace("\"", "\\\"");
 process.StartInfo.Arguments = " -offset " + 

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

2015-10-12 Thread Marek Kurdej via cfe-commits
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 wrote:
> Seems unrelated?
Oups, I've made a mistake when updating the revision and pulled in the changes 
from master.
I'll correct this.


Comment at: tools/clang-format/ClangFormat.cpp:227
@@ -213,1 +226,3 @@
+  llvm::outs() << "";
+  break;
 default:

klimek wrote:
> For XML, we should only need "<" and "&". Everything else is just important 
> in a context introduced by "<" or "&".
> The problem with include-sorting is that we'll now need to fully support utf8 
> input (well, any encoding, to be precise).
> The best way is probably to escape all code points that are outside the ascii 
> range. The problem I see is that we have no idea what encoding the file is 
> in, so I have no clue how we can really resolve this.
> 
Ok. I'll cut it  down to the necessary minimum ('<', '&').
Anyway, I thing it's more urgent to fix the include sorting containing open 
brackets and later work on UTF-8 support.


http://reviews.llvm.org/D13549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2015-10-12 Thread Marek Kurdej via cfe-commits
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/AssemblyInfo.cs
  tools/clang-format/ClangFormat.cpp

Index: tools/clang-format/ClangFormat.cpp
===
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -201,15 +201,21 @@
 static void outputReplacementXML(StringRef Text) {
   size_t From = 0;
   size_t Index;
-  while ((Index = Text.find_first_of("\n\r", From)) != StringRef::npos) {
+  while ((Index = Text.find_first_of("\n\r<&", From)) != StringRef::npos) {
 llvm::outs() << Text.substr(From, Index - From);
 switch (Text[Index]) {
 case '\n':
   llvm::outs() << "";
   break;
 case '\r':
   llvm::outs() << "";
   break;
+case '<':
+  llvm::outs() << "";
+  break;
+case '&':
+  llvm::outs() << "";
+  break;
 default:
   llvm_unreachable("Unexpected character encountered!");
 }
Index: tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
===
--- tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
+++ tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
@@ -29,5 +29,5 @@
 // You can specify all the values or you can default the Revision and Build Numbers 
 // by using the '*' as shown below:
 
-[assembly: AssemblyVersion("1.0.0.0")]
-[assembly: AssemblyFileVersion("1.0.0.0")]
+[assembly: AssemblyVersion("1.1.0.0")]
+[assembly: AssemblyFileVersion("1.1.0.0")]
Index: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
===
--- tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
+++ tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
@@ -32,13 +32,16 @@
 [CLSCompliant(false), ComVisible(true)]
 public class OptionPageGrid : DialogPage
 {
-private string style = "File";
+private string assumeFilename = "";
+private string fallbackStyle = "LLVM";
+private bool sortIncludes = false;
+private string style = "file";
 
 [Category("LLVM/Clang")]
 [DisplayName("Style")]
 [Description("Coding style, currently supports:\n" +
- "  - Predefined styles ('LLVM', 'Google', 'Chromium', 'Mozilla').\n" +
- "  - 'File' to search for a YAML .clang-format or _clang-format\n" +
+ "  - Predefined styles ('LLVM', 'Google', 'Chromium', 'Mozilla', 'WebKit').\n" +
+ "  - 'file' to search for a YAML .clang-format or _clang-format\n" +
  "configuration file.\n" +
  "  - A YAML configuration snippet.\n\n" +
  "'File':\n" +
@@ -53,6 +56,38 @@
 get { return style; }
 set { style = value; }
 }
+
+[Category("LLVM/Clang")]
+[DisplayName("Assume Filename")]
+[Description("When reading from stdin, clang-format assumes this " +
+ "filename to look for a style config file (with 'file' style) " +
+ "and to determine the language.")]
+public string AssumeFilename
+{
+get { return assumeFilename; }
+set { assumeFilename = value; }
+}
+
+[Category("LLVM/Clang")]
+[DisplayName("Fallback Style")]
+[Description("The name of the predefined style used as a fallback in case clang-format " +
+ "is invoked with 'file' style, but can not find the configuration file.\n" +
+ "Use 'none' fallback style to skip formatting.")]
+public string FallbackStyle
+{
+get { return fallbackStyle; }
+set { fallbackStyle = value; }
+}
+
+[Category("LLVM/Clang")]
+[DisplayName("Sort includes")]
+[Description("Sort touched include lines.\n\n" +
+ "See also: http://clang.llvm.org/docs/ClangFormat.html.;)]
+public bool SortIncludes
+{
+get { return sortIncludes; }
+set { sortIncludes = value; }
+}
 }
 
 [PackageRegistration(UseManagedResourcesOnly = true)]
@@ -138,10 +173,17 @@
 // Poor man's escaping - this will not work when quotes are already escaped
 // in the input (but we don't need more).
 string style = GetStyle().Replace("\"", "\\\"");
+string fallbackStyle = GetFallbackStyle().Replace("\"", "\\\"");
 process.StartInfo.Arguments = " -offset " + offset +
   " -length " + length +
   " -output-replacements-xml " +
-  

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

2015-10-12 Thread Manuel Klimek via cfe-commits
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 + "\"";
 process.StartInfo.CreateNoWindow = true;

curdeius wrote:
> 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.
I'd vote for the latter, if possible.


http://reviews.llvm.org/D13549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2015-10-12 Thread Manuel Klimek via cfe-commits
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)));
 }

Seems unrelated?


Comment at: tools/clang-format/ClangFormat.cpp:227
@@ -213,1 +226,3 @@
+  llvm::outs() << "";
+  break;
 default:

For XML, we should only need "<" and "&". Everything else is just important in 
a context introduced by "<" or "&".
The problem with include-sorting is that we'll now need to fully support utf8 
input (well, any encoding, to be precise).
The best way is probably to escape all code points that are outside the ascii 
range. The problem I see is that we have no idea what encoding the file is in, 
so I have no clue how we can really resolve this.



http://reviews.llvm.org/D13549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2015-10-12 Thread Manuel Klimek via cfe-commits
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 + "\"";
 process.StartInfo.CreateNoWindow = true;

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?)


http://reviews.llvm.org/D13549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2015-10-12 Thread Marek Kurdej via 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
  tools/clang-format/ClangFormat.cpp

Index: tools/clang-format/ClangFormat.cpp
===
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -199,17 +199,25 @@
 }
 
 static void outputReplacementXML(StringRef Text) {
+  // FIXME: When we sort includes, we need to make sure the stream is correct
+  // utf-8.
   size_t From = 0;
   size_t Index;
-  while ((Index = Text.find_first_of("\n\r", From)) != StringRef::npos) {
+  while ((Index = Text.find_first_of("\n\r<&", From)) != StringRef::npos) {
 llvm::outs() << Text.substr(From, Index - From);
 switch (Text[Index]) {
 case '\n':
   llvm::outs() << "";
   break;
 case '\r':
   llvm::outs() << "";
   break;
+case '<':
+  llvm::outs() << "";
+  break;
+case '&':
+  llvm::outs() << "";
+  break;
 default:
   llvm_unreachable("Unexpected character encountered!");
 }
Index: tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
===
--- tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
+++ tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
@@ -29,5 +29,5 @@
 // You can specify all the values or you can default the Revision and Build Numbers 
 // by using the '*' as shown below:
 
-[assembly: AssemblyVersion("1.0.0.0")]
-[assembly: AssemblyFileVersion("1.0.0.0")]
+[assembly: AssemblyVersion("1.1.0.0")]
+[assembly: AssemblyFileVersion("1.1.0.0")]
Index: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
===
--- tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
+++ tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
@@ -32,13 +32,16 @@
 [CLSCompliant(false), ComVisible(true)]
 public class OptionPageGrid : DialogPage
 {
-private string style = "File";
+private string assumeFilename = "";
+private string fallbackStyle = "LLVM";
+private bool sortIncludes = false;
+private string style = "file";
 
 [Category("LLVM/Clang")]
 [DisplayName("Style")]
 [Description("Coding style, currently supports:\n" +
- "  - Predefined styles ('LLVM', 'Google', 'Chromium', 'Mozilla').\n" +
- "  - 'File' to search for a YAML .clang-format or _clang-format\n" +
+ "  - Predefined styles ('LLVM', 'Google', 'Chromium', 'Mozilla', 'WebKit').\n" +
+ "  - 'file' to search for a YAML .clang-format or _clang-format\n" +
  "configuration file.\n" +
  "  - A YAML configuration snippet.\n\n" +
  "'File':\n" +
@@ -53,6 +56,38 @@
 get { return style; }
 set { style = value; }
 }
+
+[Category("LLVM/Clang")]
+[DisplayName("Assume Filename")]
+[Description("When reading from stdin, clang-format assumes this " +
+ "filename to look for a style config file (with 'file' style) " +
+ "and to determine the language.")]
+public string AssumeFilename
+{
+get { return assumeFilename; }
+set { assumeFilename = value; }
+}
+
+[Category("LLVM/Clang")]
+[DisplayName("Fallback Style")]
+[Description("The name of the predefined style used as a fallback in case clang-format " +
+ "is invoked with 'file' style, but can not find the configuration file.\n" +
+ "Use 'none' fallback style to skip formatting.")]
+public string FallbackStyle
+{
+get { return fallbackStyle; }
+set { fallbackStyle = value; }
+}
+
+[Category("LLVM/Clang")]
+[DisplayName("Sort includes")]
+[Description("Sort touched include lines.\n\n" +
+ "See also: http://clang.llvm.org/docs/ClangFormat.html.;)]
+public bool SortIncludes
+{
+get { return sortIncludes; }
+set { sortIncludes = value; }
+}
 }
 
 [PackageRegistration(UseManagedResourcesOnly = true)]
@@ -138,10 +173,17 @@
 // Poor man's escaping - this will not work when quotes are already escaped
 // in the input (but we don't need more).
 string style = GetStyle().Replace("\"", "\\\"");
+string fallbackStyle = GetFallbackStyle().Replace("\"", "\\\"");
 process.StartInfo.Arguments = " -offset " + offset +
   " -length " + 

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

2015-10-12 Thread Marek Kurdej via cfe-commits
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:
> 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.


http://reviews.llvm.org/D13549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2015-10-12 Thread Manuel Klimek via cfe-commits
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 important 
> > in a context introduced by "<" or "&".
> > The problem with include-sorting is that we'll now need to fully support 
> > utf8 input (well, any encoding, to be precise).
> > The best way is probably to escape all code points that are outside the 
> > ascii range. The problem I see is that we have no idea what encoding the 
> > file is in, so I have no clue how we can really resolve this.
> > 
> Ok. I'll cut it  down to the necessary minimum ('<', '&').
> Anyway, I thing it's more urgent to fix the include sorting containing open 
> brackets and later work on UTF-8 support.
Agreed. Can you please add a FIXME though:
// FIXME: When we sort includes, we need to make sure the stream is correct
// utf-8.


http://reviews.llvm.org/D13549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2015-10-12 Thread Marek Kurdej via cfe-commits
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


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

2015-10-12 Thread Marek Kurdej via cfe-commits
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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2015-10-12 Thread Marek Kurdej via cfe-commits
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://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2015-10-12 Thread Marek Kurdej via cfe-commits
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-vs/ClangFormat/Properties/AssemblyInfo.cs
  tools/clang-format/ClangFormat.cpp

Index: tools/clang-format/ClangFormat.cpp
===
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -199,17 +199,25 @@
 }
 
 static void outputReplacementXML(StringRef Text) {
+  // FIXME: When we sort includes, we need to make sure the stream is correct
+  // utf-8.
   size_t From = 0;
   size_t Index;
-  while ((Index = Text.find_first_of("\n\r", From)) != StringRef::npos) {
+  while ((Index = Text.find_first_of("\n\r<&", From)) != StringRef::npos) {
 llvm::outs() << Text.substr(From, Index - From);
 switch (Text[Index]) {
 case '\n':
   llvm::outs() << "";
   break;
 case '\r':
   llvm::outs() << "";
   break;
+case '<':
+  llvm::outs() << "";
+  break;
+case '&':
+  llvm::outs() << "";
+  break;
 default:
   llvm_unreachable("Unexpected character encountered!");
 }
Index: tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
===
--- tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
+++ tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
@@ -29,5 +29,5 @@
 // You can specify all the values or you can default the Revision and Build Numbers 
 // by using the '*' as shown below:
 
-[assembly: AssemblyVersion("1.0.0.0")]
-[assembly: AssemblyFileVersion("1.0.0.0")]
+[assembly: AssemblyVersion("1.1.0.0")]
+[assembly: AssemblyFileVersion("1.1.0.0")]
Index: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
===
--- tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
+++ tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
@@ -19,6 +19,7 @@
 using Microsoft.VisualStudio.Text.Editor;
 using Microsoft.VisualStudio.TextManager.Interop;
 using System;
+using System.Collections;
 using System.ComponentModel;
 using System.ComponentModel.Design;
 using System.IO;
@@ -32,13 +33,53 @@
 [CLSCompliant(false), ComVisible(true)]
 public class OptionPageGrid : DialogPage
 {
-private string style = "File";
+private string assumeFilename = "";
+private string fallbackStyle = "LLVM";
+private bool sortIncludes = false;
+private string style = "file";
+
+public class StyleConverter : TypeConverter
+{
+protected ArrayList values;
+public StyleConverter()
+{
+// Initializes the standard values list with defaults.
+values = new ArrayList(new string[] { "file", "Chromium", "Google", "LLVM", "Mozilla", "WebKit" });
+}
+
+public override bool GetStandardValuesSupported(ITypeDescriptorContext context)
+{
+return true;
+}
+
+public override StandardValuesCollection GetStandardValues(ITypeDescriptorContext context)
+{
+StandardValuesCollection svc = new StandardValuesCollection(values);
+return svc;
+}
+
+public override bool CanConvertFrom(ITypeDescriptorContext context, Type sourceType)
+{
+if (sourceType == typeof(string))
+return true;
+else 
+return base.CanConvertFrom(context, sourceType);
+}
+
+public override object ConvertFrom(ITypeDescriptorContext context, System.Globalization.CultureInfo culture, object value)
+{
+if (value.GetType() == typeof(string))
+return value;
+else
+return base.ConvertFrom(context, culture, value);
+}
+}
 
 [Category("LLVM/Clang")]
 [DisplayName("Style")]
 [Description("Coding style, currently supports:\n" +
- "  - Predefined styles ('LLVM', 'Google', 'Chromium', 'Mozilla').\n" +
- "  - 'File' to search for a YAML .clang-format or _clang-format\n" +
+ "  - Predefined styles ('LLVM', 'Google', 'Chromium', 'Mozilla', 'WebKit').\n" +
+ "  - 'file' to search for a YAML .clang-format or _clang-format\n" +
  "configuration file.\n" +
  "  - A YAML configuration snippet.\n\n" +
  "'File':\n" +
@@ -48,11 +89,79 @@
  "  The content of a .clang-format configuration file, as string.\n" +
  "  Example: '{BasedOnStyle: \"LLVM\", 

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

2015-10-08 Thread Daniel Jasper via cfe-commits
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-bin/mailman/listinfo/cfe-commits


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

2015-10-08 Thread Marek Kurdej via cfe-commits
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.cpp

Index: tools/clang-format/ClangFormat.cpp
===
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -201,15 +201,30 @@
 static void outputReplacementXML(StringRef Text) {
   size_t From = 0;
   size_t Index;
-  while ((Index = Text.find_first_of("\n\r", From)) != StringRef::npos) {
+  while ((Index = Text.find_first_of("\n\r<>&\"'", From)) != StringRef::npos) {
 llvm::outs() << Text.substr(From, Index - From);
 switch (Text[Index]) {
 case '\n':
   llvm::outs() << "";
   break;
 case '\r':
   llvm::outs() << "";
   break;
+case '<':
+  llvm::outs() << "";
+  break;
+case '>':
+  llvm::outs() << "";
+  break;
+case '&':
+  llvm::outs() << "";
+  break;
+case '\'':
+  llvm::outs() << "";
+  break;
+case '"':
+  llvm::outs() << "";
+  break;
 default:
   llvm_unreachable("Unexpected character encountered!");
 }
Index: tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
===
--- tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
+++ tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs
@@ -29,5 +29,5 @@
 // You can specify all the values or you can default the Revision and Build Numbers 
 // by using the '*' as shown below:
 
-[assembly: AssemblyVersion("1.0.0.0")]
-[assembly: AssemblyFileVersion("1.0.0.0")]
+[assembly: AssemblyVersion("1.1.0.0")]
+[assembly: AssemblyFileVersion("1.1.0.0")]
Index: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
===
--- tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
+++ tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs
@@ -32,13 +32,16 @@
 [CLSCompliant(false), ComVisible(true)]
 public class OptionPageGrid : DialogPage
 {
-private string style = "File";
+private string assumeFilename = "";
+private string fallbackStyle = "LLVM";
+private bool sortIncludes = false;
+private string style = "file";
 
 [Category("LLVM/Clang")]
 [DisplayName("Style")]
 [Description("Coding style, currently supports:\n" +
- "  - Predefined styles ('LLVM', 'Google', 'Chromium', 'Mozilla').\n" +
- "  - 'File' to search for a YAML .clang-format or _clang-format\n" +
+ "  - Predefined styles ('LLVM', 'Google', 'Chromium', 'Mozilla', 'WebKit').\n" +
+ "  - 'file' to search for a YAML .clang-format or _clang-format\n" +
  "configuration file.\n" +
  "  - A YAML configuration snippet.\n\n" +
  "'File':\n" +
@@ -53,6 +56,38 @@
 get { return style; }
 set { style = value; }
 }
+
+[Category("LLVM/Clang")]
+[DisplayName("Assume Filename")]
+[Description("When reading from stdin, clang-format assumes this " +
+ "filename to look for a style config file (with 'file' style) \n" +
+ "and to determine the language.")]
+public string AssumeFilename
+{
+get { return assumeFilename; }
+set { assumeFilename = value; }
+}
+
+[Category("LLVM/Clang")]
+[DisplayName("Fallback Style")]
+[Description("The name of the predefined style used as a fallback in case clang-format " +
+ "is invoked with 'file' style, but can not find the configuration file.\n" +
+ "Use 'none' fallback style to skip formatting.")]
+public string FallbackStyle
+{
+get { return fallbackStyle; }
+set { fallbackStyle = value; }
+}
+
+[Category("LLVM/Clang")]
+[DisplayName("Sort includes")]
+[Description("Sort touched include lines.\n\n" +
+ "See also: http://clang.llvm.org/docs/ClangFormat.html.;)]
+public bool SortIncludes
+{
+get { return sortIncludes; }
+set { sortIncludes = value; }
+}
 }
 
 [PackageRegistration(UseManagedResourcesOnly = true)]
@@ -138,10 +173,17 @@
 // Poor man's escaping - this will not work when quotes are already escaped
 // in the input (but we don't need more).
 string style = GetStyle().Replace("\"", "\\\"");
+string fallbackStyle = GetFallbackStyle().Replace("\"", "\\\"");
 process.StartInfo.Arguments = " -offset " + offset +