Re: [PATCH] D13549: Added new options to ClangFormat VSIX package.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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 +