Commit 57599ef1 authored by Jon Skeet's avatar Jon Skeet

A few stylistic issues

parent c58ce5dc
......@@ -266,10 +266,10 @@
<property name="Configuration" value="Release" />
<property name="Platform" value="Any CPU" />
</msbuild>
<!--msbuild project="${src}/ProtocolBuffers.sln">
<msbuild project="${src}/ProtocolBuffers.sln">
<property name="Configuration" value="Silverlight2" />
<property name="Platform" value="Any CPU" />
</msbuild-->
</msbuild>
<!-- Note the deliberate lack of space in the platform name -->
<msbuild project="${src}/ProtocolBuffers/ProtocolBuffersCF.csproj">
<property name="Configuration" value="Release" />
......@@ -291,7 +291,7 @@
<mkdir dir="${output-dir}/Protoc" />
<mkdir dir="${output-dir}/Debug" />
<mkdir dir="${output-dir}/Release" />
<!--mkdir dir="${output-dir}/Silverlight2" /-->
<mkdir dir="${output-dir}/Silverlight2" />
<mkdir dir="${output-dir}/CompactFramework35" />
<copy todir="${output-dir}/Protoc">
<fileset basedir="${project::get-base-directory()}/lib">
......@@ -321,12 +321,12 @@
<exclude name="**/*vshost*" />
</fileset>
</copy>
<!--copy todir="${output-dir}/Silverlight2"
<copy todir="${output-dir}/Silverlight2"
flatten="true">
<fileset basedir="${src}">
<include name="ProtocolBuffers/bin/Silverlight2/Google.ProtocolBuffers.dll" />
</fileset>
</copy-->
</copy>
<copy todir="${output-dir}/CompactFramework35"
flatten="true">
<fileset basedir="${src}">
......
......@@ -39,7 +39,6 @@ message CSharpFileOptions {
// Generate attributes indicating non-CLS-compliance
optional bool cls_compliance = 8 [default = true];
// ROK - 2010-09-03 additions to csoptions ...
// The extension that should be appended to the umbrella_classname when creating files.
optional string file_extension = 221 [default = ".cs"];
......
......@@ -50,7 +50,7 @@ namespace Google.ProtocolBuffers.ProtoGen {
FileDescriptorProto second = new FileDescriptorProto.Builder { Name="Second" }.Build();
FileDescriptorSet set = new FileDescriptorSet { FileList = { first, second } };
IList<FileDescriptor> converted = Generator.ConvertDescriptors(set);
IList<FileDescriptor> converted = Generator.ConvertDescriptors(CSharpFileOptions.DefaultInstance, set);
Assert.AreEqual(2, converted.Count);
Assert.AreEqual("First", converted[0].Name);
Assert.AreEqual(0, converted[0].Dependencies.Count);
......@@ -63,7 +63,7 @@ namespace Google.ProtocolBuffers.ProtoGen {
FileDescriptorProto first = new FileDescriptorProto.Builder { Name = "First", DependencyList = {"Second"} }.Build();
FileDescriptorProto second = new FileDescriptorProto.Builder { Name = "Second" }.Build();
FileDescriptorSet set = new FileDescriptorSet { FileList = { first, second } };
IList<FileDescriptor> converted = Generator.ConvertDescriptors(set);
IList<FileDescriptor> converted = Generator.ConvertDescriptors(CSharpFileOptions.DefaultInstance, set);
Assert.AreEqual(2, converted.Count);
Assert.AreEqual("First", converted[0].Name);
Assert.AreEqual(1, converted[0].Dependencies.Count);
......@@ -77,7 +77,7 @@ namespace Google.ProtocolBuffers.ProtoGen {
FileDescriptorProto first = new FileDescriptorProto.Builder { Name = "First" }.Build();
FileDescriptorProto second = new FileDescriptorProto.Builder { Name = "Second", DependencyList = {"First"} }.Build();
FileDescriptorSet set = new FileDescriptorSet { FileList = { first, second } };
IList<FileDescriptor> converted = Generator.ConvertDescriptors(set);
IList<FileDescriptor> converted = Generator.ConvertDescriptors(CSharpFileOptions.DefaultInstance, set);
Assert.AreEqual(2, converted.Count);
Assert.AreEqual("First", converted[0].Name);
Assert.AreEqual(0, converted[0].Dependencies.Count);
......@@ -92,7 +92,7 @@ namespace Google.ProtocolBuffers.ProtoGen {
FileDescriptorProto second = new FileDescriptorProto.Builder { Name = "Second", DependencyList = { "First" } }.Build();
FileDescriptorSet set = new FileDescriptorSet { FileList = { first, second } };
try {
Generator.ConvertDescriptors(set);
Generator.ConvertDescriptors(CSharpFileOptions.DefaultInstance, set);
Assert.Fail("Expected exception");
} catch (DependencyResolutionException) {
// Expected
......@@ -104,7 +104,7 @@ namespace Google.ProtocolBuffers.ProtoGen {
FileDescriptorProto first = new FileDescriptorProto.Builder { Name = "First", DependencyList = { "Second" } }.Build();
FileDescriptorSet set = new FileDescriptorSet { FileList = { first } };
try {
Generator.ConvertDescriptors(set);
Generator.ConvertDescriptors(CSharpFileOptions.DefaultInstance, set);
Assert.Fail("Expected exception");
} catch (DependencyResolutionException) {
// Expected
......@@ -116,7 +116,7 @@ namespace Google.ProtocolBuffers.ProtoGen {
FileDescriptorProto first = new FileDescriptorProto.Builder { Name = "First", DependencyList = { "First" } }.Build();
FileDescriptorSet set = new FileDescriptorSet { FileList = { first } };
try {
Generator.ConvertDescriptors(set);
Generator.ConvertDescriptors(CSharpFileOptions.DefaultInstance, set);
Assert.Fail("Expected exception");
} catch (DependencyResolutionException) {
// Expected
......
......@@ -5,44 +5,50 @@ using System.Text;
namespace Google.ProtocolBuffers.ProtoGen
{
class ProtoFile : TempFile
{
public ProtoFile (string filename, string contents)
{
_tempFile = filename;
File.WriteAllText(_tempFile, contents);
}
}
class TempFile : IDisposable
{
protected string _tempFile;
class ProtoFile : TempFile
{
public ProtoFile(string filename, string contents)
: base(filename, contents)
{
}
}
class TempFile : IDisposable
{
private string tempFile;
public static TempFile Attach(string path)
{
TempFile f = new TempFile();
f._tempFile = path;
return f;
}
public static TempFile Attach(string path)
{
return new TempFile(path, null);
}
protected TempFile() { }
public TempFile(string contents)
{
File.WriteAllText(_tempFile = Path.GetTempFileName(), contents, Encoding.ASCII);
}
protected TempFile(string filename, string contents) {
tempFile = filename;
if (contents != null)
{
File.WriteAllText(tempFile, contents, new UTF8Encoding(false));
}
}
public string TempPath { get { return _tempFile; } }
public TempFile(string contents)
: this(Path.GetTempFileName(), contents)
{
}
public void ChangeExtension(string ext)
{
string newFile = Path.ChangeExtension(_tempFile, ext);
File.Move(_tempFile, newFile);
_tempFile = newFile;
}
public string TempPath { get { return tempFile; } }
public void Dispose()
{
if(File.Exists(_tempFile))
File.Delete(_tempFile);
}
}
public void ChangeExtension(string ext)
{
string newFile = Path.ChangeExtension(tempFile, ext);
File.Move(tempFile, newFile);
tempFile = newFile;
}
public void Dispose()
{
if (File.Exists(tempFile))
{
File.Delete(tempFile);
}
}
}
}
This diff is collapsed.
......@@ -45,24 +45,25 @@ namespace Google.ProtocolBuffers.ProtoGen {
internal static string GetFullUmbrellaClassName(IDescriptor descriptor) {
CSharpFileOptions options = descriptor.File.CSharpOptions;
string result = options.Namespace;
if (result != "") result += '.';
result += QualifiedUmbrellaClassName(options);
if (result != "") {
result += '.';
}
result += GetQualifiedUmbrellaClassName(options);
return "global::" + result;
}
/// <summary>
/// ROK 2010-09-03
/// Evaluates the options and returns the qualified name of the umbrella class
/// relative to the descriptor type's namespace. Basically contacts the
/// UmbrellaNamespace + UmbrellaClassname fields.
/// relative to the descriptor type's namespace. Basically concatenates the
/// UmbrellaNamespace + UmbrellaClassname fields.
/// </summary>
internal static string QualifiedUmbrellaClassName(CSharpFileOptions options)
{
string fullName = options.UmbrellaClassname;
if (!options.NestClasses && options.UmbrellaNamespace != "")
fullName = String.Format("{0}.{1}", options.UmbrellaNamespace, options.UmbrellaClassname);
return fullName;
}
internal static string GetQualifiedUmbrellaClassName(CSharpFileOptions options) {
string fullName = options.UmbrellaClassname;
if (!options.NestClasses && options.UmbrellaNamespace != "") {
fullName = String.Format("{0}.{1}", options.UmbrellaNamespace, options.UmbrellaClassname);
}
return fullName;
}
internal static string GetMappedTypeName(MappedType type) {
switch(type) {
......
......@@ -60,30 +60,30 @@ namespace Google.ProtocolBuffers.ProtoGen {
}
public void Generate() {
List<FileDescriptorSet> descriptorProtos = new List<FileDescriptorSet>();
List<FileDescriptorSet> descriptorProtos = new List<FileDescriptorSet>();
foreach (string inputFile in options.InputFiles) {
ExtensionRegistry extensionRegistry = ExtensionRegistry.CreateInstance();
extensionRegistry.Add(CSharpOptions.CSharpFileOptions);
extensionRegistry.Add(CSharpOptions.CSharpFieldOptions);
using (Stream inputStream = File.OpenRead(inputFile)) {
descriptorProtos.Add(FileDescriptorSet.ParseFrom(inputStream, extensionRegistry));
descriptorProtos.Add(FileDescriptorSet.ParseFrom(inputStream, extensionRegistry));
}
}
}
IList<FileDescriptor> descriptors = ConvertDescriptors(options.FileOptions, descriptorProtos.ToArray());
IList<FileDescriptor> descriptors = ConvertDescriptors(options.FileOptions, descriptorProtos.ToArray());
//ROK Combine with Options...
foreach (FileDescriptor descriptor in descriptors)
descriptor.ConfigureWithDefaultOptions(options.FileOptions);
foreach (FileDescriptor descriptor in descriptors) {
//ROK 2010-09-03 Ignore google protobuf package
if(descriptor.CSharpOptions.IgnoreGoogleProtobuf && descriptor.Package == "google.protobuf")
continue;
// Combine with options from command line
foreach (FileDescriptor descriptor in descriptors) {
descriptor.ConfigureWithDefaultOptions(options.FileOptions);
}
Generate(descriptor);
foreach (FileDescriptor descriptor in descriptors) {
// Optionally exclude descriptors in google.protobuf
if (descriptor.CSharpOptions.IgnoreGoogleProtobuf && descriptor.Package == "google.protobuf") {
continue;
}
Generate(descriptor);
}
}
......@@ -102,13 +102,9 @@ namespace Google.ProtocolBuffers.ProtoGen {
private string GetOutputFile(FileDescriptor descriptor) {
CSharpFileOptions fileOptions = descriptor.CSharpOptions;
//ROK 2010-09-03 - added the ability to sepcify the extension used within the options
//string filename = descriptor.CSharpOptions.UmbrellaClassname + ".cs";
string filename = descriptor.CSharpOptions.UmbrellaClassname + descriptor.CSharpOptions.FileExtension;
string filename = descriptor.CSharpOptions.UmbrellaClassname + descriptor.CSharpOptions.FileExtension;
//ROK 2010-09-03 - output directory can be specific to a descriptor file
//string outputDirectory = options.OutputDirectory;
string outputDirectory = descriptor.CSharpOptions.OutputDirectory;
string outputDirectory = descriptor.CSharpOptions.OutputDirectory;
if (fileOptions.ExpandNamespaceDirectories) {
string package = fileOptions.Namespace;
if (!string.IsNullOrEmpty(package)) {
......@@ -117,29 +113,25 @@ namespace Google.ProtocolBuffers.ProtoGen {
outputDirectory = Path.Combine(outputDirectory, bit);
}
}
}
//ROK 2010-09-03 - Always force output directory exists since they can specify this in .proto options
Directory.CreateDirectory(outputDirectory);
}
// As the directory can be explicitly specified in options, we need to make sure it exists
Directory.CreateDirectory(outputDirectory);
return Path.Combine(outputDirectory, filename);
}
// ROK 2010-09-03 - used by unit tests, we will continue to allow them to function as-is.
internal static IList<FileDescriptor> ConvertDescriptors(FileDescriptorSet descriptorProtos) {
return ConvertDescriptors(CSharpFileOptions.DefaultInstance, descriptorProtos);
}
/// <summary>
/// Resolves any dependencies and converts FileDescriptorProtos into FileDescriptors.
/// The list returned is in the same order as the protos are listed in the descriptor set.
/// Note: this method is internal rather than private to allow testing.
/// </summary>
/// <exception cref="DependencyResolutionException">Not all dependencies could be resolved.</exception>
private static IList<FileDescriptor> ConvertDescriptors(CSharpFileOptions options, params FileDescriptorSet[] descriptorProtos) {
internal static IList<FileDescriptor> ConvertDescriptors(CSharpFileOptions options, params FileDescriptorSet[] descriptorProtos) {
// Simple strategy: Keep going through the list of protos to convert, only doing ones where
// we've already converted all the dependencies, until we get to a stalemate
List<FileDescriptorProto> fileList = new List<FileDescriptorProto>();
foreach (FileDescriptorSet set in descriptorProtos)
fileList.AddRange(set.FileList);
foreach (FileDescriptorSet set in descriptorProtos)
fileList.AddRange(set.FileList);
FileDescriptor[] converted = new FileDescriptor[fileList.Count];
......@@ -158,31 +150,28 @@ namespace Google.ProtocolBuffers.ProtoGen {
FileDescriptorProto candidate = fileList[i];
FileDescriptor[] dependencies = new FileDescriptor[candidate.DependencyList.Count];
CSharpFileOptions.Builder builder = options.ToBuilder();
if (candidate.Options.HasExtension(DescriptorProtos.CSharpOptions.CSharpFileOptions)) {
builder.MergeFrom(candidate.Options.GetExtension(DescriptorProtos.CSharpOptions.CSharpFileOptions));
}
CSharpFileOptions.Builder builder = options.ToBuilder();
if (candidate.Options.HasExtension(DescriptorProtos.CSharpOptions.CSharpFileOptions)) {
builder.MergeFrom(candidate.Options.GetExtension(DescriptorProtos.CSharpOptions.CSharpFileOptions));
}
CSharpFileOptions localOptions = builder.Build();
bool foundAllDependencies = true;
for (int j = 0; j < dependencies.Length; j++) {
if (!convertedMap.TryGetValue(candidate.DependencyList[j], out dependencies[j])) {
// ROK 2010-09-03 - we can auto-magically resolve these since we already have their description
// This way if the file is only referencing options it does not need to be built with the
// --include_imports definition.
if (localOptions.IgnoreGoogleProtobuf && (candidate.DependencyList[j] == "google/protobuf/csharp_options.proto"))
{
dependencies[j] = CSharpOptions.Descriptor;
continue;
}
if (localOptions.IgnoreGoogleProtobuf && (candidate.DependencyList[j] == "google/protobuf/descriptor.proto"))
{
dependencies[j] = DescriptorProtoFile.Descriptor;
continue;
}
foundAllDependencies = false;
// We can auto-magically resolve these since we already have their description
// This way if the file is only referencing options it does not need to be built with the
// --include_imports definition.
if (localOptions.IgnoreGoogleProtobuf && (candidate.DependencyList[j] == "google/protobuf/csharp_options.proto")) {
dependencies[j] = CSharpOptions.Descriptor;
continue;
}
if (localOptions.IgnoreGoogleProtobuf && (candidate.DependencyList[j] == "google/protobuf/descriptor.proto")) {
dependencies[j] = DescriptorProtoFile.Descriptor;
continue;
}
foundAllDependencies = false;
break;
}
}
......
This diff is collapsed.
#region Copyright notice and license
// Protocol Buffers - Google's data interchange format
// Copyright 2008 Google Inc. All rights reserved.
// http://github.com/jskeet/dotnet-protobufs/
......@@ -30,6 +31,7 @@
// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#endregion
using System;
......@@ -40,12 +42,12 @@ namespace Google.ProtocolBuffers.ProtoGen {
/// <summary>
/// Entry point for the Protocol Buffers generator.
/// </summary>
class Program {
internal static int Main(string[] args) {
internal class Program {
internal static int Main(string[] args) {
try {
// Hack to make sure everything's initialized
DescriptorProtoFile.Descriptor.ToString();
GeneratorOptions options = ParseCommandLineArguments(args);
GeneratorOptions options = new GeneratorOptions {Arguments = args};
IList<string> validationFailures;
if (!options.TryValidate(out validationFailures)) {
......@@ -58,25 +60,13 @@ namespace Google.ProtocolBuffers.ProtoGen {
Generator generator = Generator.CreateGenerator(options);
generator.Generate();
return 0;
} catch (Exception e) {
}
catch (Exception e) {
Console.Error.WriteLine("Error: {0}", e.Message);
Console.Error.WriteLine();
Console.Error.WriteLine("Detailed exception information: {0}", e);
return 1;
}
}
private static GeneratorOptions ParseCommandLineArguments(string[] args) {
GeneratorOptions options = new GeneratorOptions();
//string baseDir = "c:\\Users\\Jon\\Documents\\Visual Studio 2008\\Projects\\ProtocolBuffers";
//options.OutputDirectory = baseDir + "\\tmp";
//options.InputFiles = new[] { baseDir + "\\protos\\nwind-solo.protobin" };
//ROK 2010-09-03 - fixes to allow parsing these options...
//options.OutputDirectory = ".";
//options.InputFiles = args;
options.Arguments = args;
return options;
}
}
}
\ No newline at end of file
This diff is collapsed.
......@@ -61,7 +61,7 @@ namespace Google.ProtocolBuffers.ProtoGen {
writer.WriteLine();
writer.WriteLine("{0} static pbd::ServiceDescriptor Descriptor {{", ClassAccessLevel);
writer.WriteLine(" get {{ return {0}.Descriptor.Services[{1}]; }}",
DescriptorUtil.QualifiedUmbrellaClassName(Descriptor.File.CSharpOptions), Descriptor.Index);
DescriptorUtil.GetQualifiedUmbrellaClassName(Descriptor.File.CSharpOptions), Descriptor.Index);
writer.WriteLine("}");
writer.WriteLine("{0} pbd::ServiceDescriptor DescriptorForType {{", ClassAccessLevel);
writer.WriteLine(" get { return Descriptor; }");
......
......@@ -93,12 +93,12 @@ namespace Google.ProtocolBuffers.ProtoGen {
writer.Outdent();
writer.WriteLine("}");
//ROK 2010-09-03 - close the namespace around the umbrella class if defined
if (!Descriptor.CSharpOptions.NestClasses && Descriptor.CSharpOptions.UmbrellaNamespace != "") {
writer.Outdent();
writer.WriteLine("}");
}
}
// Close the namespace around the umbrella class if defined
if (!Descriptor.CSharpOptions.NestClasses && Descriptor.CSharpOptions.UmbrellaNamespace != "") {
writer.Outdent();
writer.WriteLine("}");
}
}
WriteChildren(writer, "Enums", Descriptor.EnumTypes);
WriteChildren(writer, "Messages", Descriptor.MessageTypes);
WriteChildren(writer, "Services", Descriptor.Services);
......@@ -122,12 +122,12 @@ namespace Google.ProtocolBuffers.ProtoGen {
writer.Indent();
writer.WriteLine();
}
//ROK 2010-09-03 - add the namespace around the umbrella class if defined
if(!Descriptor.CSharpOptions.NestClasses && Descriptor.CSharpOptions.UmbrellaNamespace != "") {
// Add the namespace around the umbrella class if defined
if(!Descriptor.CSharpOptions.NestClasses && Descriptor.CSharpOptions.UmbrellaNamespace != "") {
writer.WriteLine("namespace {0} {{", Descriptor.CSharpOptions.UmbrellaNamespace);
writer.Indent();
writer.WriteLine();
}
}
if (Descriptor.CSharpOptions.CodeContracts) {
writer.WriteLine("[global::System.Diagnostics.Contracts.ContractVerificationAttribute(false)]");
......
......@@ -76,7 +76,7 @@ namespace Google.ProtocolBuffers {
public void DynamicMessageSettersRejectNull() {
IBuilder builder = DynamicMessage.CreateBuilder(TestAllTypes.Descriptor);
reflectionTester.AssertReflectionSettersRejectNull(builder);
}
}
[Test]
public void DynamicMessageExtensionAccessors() {
......@@ -93,7 +93,7 @@ namespace Google.ProtocolBuffers {
public void DynamicMessageExtensionSettersRejectNull() {
IBuilder builder = DynamicMessage.CreateBuilder(TestAllExtensions.Descriptor);
extensionsReflectionTester.AssertReflectionSettersRejectNull(builder);
}
}
[Test]
public void DynamicMessageRepeatedSetters() {
......
......@@ -335,7 +335,7 @@ namespace Google.ProtocolBuffers {
}
public override bool IsInitialized {
get { return fields.IsInitializedWithRespectTo(type); }
get { return fields.IsInitializedWithRespectTo(type); }
}
public override Builder MergeFrom(CodedInputStream input, ExtensionRegistry extensionRegistry) {
......
......@@ -273,7 +273,7 @@ namespace Google.ProtocolBuffers {
if (extras.Count == 0) {
return current;
}
if (current == null) {
if (current == null) {
current = new List<T>(extras);
} else {
current.AddRange(extras);
......@@ -359,7 +359,7 @@ namespace Google.ProtocolBuffers {
/// <returns></returns>
private static ReadOnlyCollection<T> MakeReadOnly<T>(ref List<T> list) {
ReadOnlyCollection<T> ret = list == null ? Lists<T>.Empty : new ReadOnlyCollection<T>(list);
list = null;
list = null;
return ret;
}
}
......
Work complete on csharptest/branch
New Options (in csharp_options.proto):
1 - Add a way to specify the output directory
Added an option to csharp_options.proto called "output_directory", defaults to "."
2 - Added an option "file_extension" to control the suffix for cs files generated, defaults to ".cs"
This enables using ".Generated.cs" to provide easy filtering and cleanup of generated code.
3 - Added the option for "umbrella_namespace" used when nest_classes=false and having name conflicts
4 - Optionally remove dependencies to csharp options
provided option "ignore_google_protobuf" to prevent generation of code for csharp_options.proto and descriptor.proto
option also relaxes the contraint of having either of these compiled into the proto buffer input
5 - Investigate command line parsing library
All proto options for csharp_options.proto can now be provided via the command-line by using the following format:
/option=value or -option:value
i.e. use /namespace=My.Name.Space
6 - Investigate calling protoc directly
ProgramPreprocess.cs - input files with an extension of '.proto' by running protoc.exe. If arguments
are supplied with '--' prefix they are provided to protoc.exe, otherwise they are assumed to
be used for ProtoGen.exe which is run on the resulting output proto buffer. If the option
--descriptor_set_out= is specified the proto buffer file is kept, otherwise it will be removed
after code generation.
7 - Unable to resolve dependencies correctly
Fixed an issue where two or more proto-buffers are provided to protogen.exe that depend on types
defined in one-another. The dependency walker of the generator was not taking into account all
inputs when evaluating the descriptor's existence.
8 - Added several (20) nunits to automate the command-line invocation of each option and ensured the
generated code compiles into a working assembly and contains some of the expected types. Not sure
how to disable this test for mono, I'm certain it will not work; however, I still believe it's worth
having. As a side-benefit from testing the command-line options, it verifies behavior of each
setting in csharp_options (save for multi-file).
Current task list (not in order)
? Optionally remove dependencies to csharp options
- Remove multifile support
- Docs
- Clean up protogen code
X Add flags to protogen
- Avoid using reflection for messages which don't need it (is this
possible?)
- Bring service generation into line with Java
......@@ -53,7 +12,5 @@ X Add flags to protogen
- Reformat code
- Change generated format
- Add regions to copyright
X Investigate command line parsing library
X Investigate calling protoc directly
- Build and publish binaries
- Work out why the Compact Framework 3.5 build fails under VS2010
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment