C# Transpiler, API, and Tests #129

Merged
pragmatrix merged 1 commits from csharp-fb-pr into master 2015-09-23 09:08:15 -07:00
pragmatrix commented 2015-09-23 01:27:01 -07:00 (Migrated from github.com)

This pull request adds a C#/.NET transpiler, API, and tests to the css-layout project. The transpiler is integrated by adding a few lines to transpile.js and the API is very similar to the Java version.

Details

Formatting

Even though the contribution guide states otherwise, the C# code is indented by 4 spaces instead of 2, and { braces are usually placed on the next line. I think that keeping the default Visual Studio settings for C# code will make the code easier to maintain, also I've added an .editorconfig file so that other editors can pick up the correct format.

Idiomatic CSharp

All public names of classes, methods, and members are in PascalCase. Properties were used when they seemed appropriate.

Enum members defined by CSSAlign, CSSConstants, CSSDirection, CSSFlexDirection, CSSJustify, CSSPositionType, and CSSWrap have been converted to PascalCase. CSharpTranspiler.js will take care of the case conversion for the generated code.

Method forwarders / duplication

To keep the differences between the Java and C# transpiler minimal, I decided to leave some internal methods in camlCase. When appropriate, these methods were implemented as extension methods.

Visual Studio Solution

The solution's CSSLayout project is located in src/csharp and produces a portable class library Profile 259, which supports the .NET Framework 4.5, Windows 8+, Windows Phone 8.1, Windows Phone Silverlight 8, and Xamarin.Android, Xamarin.iOS, and Xamarin.iOS (Classic API).

The solution also includes the test project, which uses NUnit.

The code uses C# 6 syntax.

File Format

All *.cs files are encoded in UTF-8, LF line endings, no BOMs. *.sln, *.csproj, and *.config files are in UTF-8 with BOMs and CRLF line endings.

File Notes

CSSNode.cs

  • All methods are non-virtual so far. This is because I don't know which methods are intended to be virtual and which are not, so feedback is very appreciated here. For the projects I've done with the C# version, inheriting from CSSNode was not required.
  • The names of the properties for accessing the style width and height are named just Width and Height instead of StyleWidth and StyleHeight. All properties except the ones beginning with Layout are meant to be style related.
  • I've added Min/Max-Height/Width properties to CSSNode for reading and modifying CSSStyle fields because the CSSNode.style member is not accessible through CSSNode yet.
  • The measure function is defined as a .NET delegate.

Spacing.cs

This class seems not to be indented to be public, but gets exposed in CSSNode in the Java API through a field named padding. I decided to remove CSSNode.padding for now, because padding can already be modified with CSSNode's methods.

CSSSpacingType.cs

I did not liked the idea of using integers for the spacing type, so I've introduced an enum named CSSSpacingType, which is used in CSSNode for specifying the various types of spacing for paddings, margins, and borders.

Build Script

Only the transpiler is run from the gruntfile. However, there is a Makefile in the src/csharp directory that is used to build & test the solution and pack & upload the NuGet package.

NuGet Package

This pull request is based on an unofficial adoption, which I am maintaining for some time. Also there is a NuGet package available that is named Facebook.CSSLayout. I am happy to transfer ownership, if this pull request is accepted.

I've decided to properly clean up the API before creating this pull request, thereby introducing (hopefully very minor) incompatibilities with code written against the NuGet packages.

And sorry, no history, I've never rebased the csharp version, but will from now on.

This pull request adds a C#/.NET transpiler, API, and tests to the css-layout project. The transpiler is integrated by adding a few lines to transpile.js and the API is very similar to the Java version. ## Details ### Formatting Even though the contribution guide states otherwise, the C# code is indented by 4 spaces instead of 2, and `{` braces are usually placed on the next line. I think that keeping the default Visual Studio settings for C# code will make the code easier to maintain, also I've added an `.editorconfig` file so that other editors can pick up the correct format. ### Idiomatic CSharp All public names of classes, methods, and members are in PascalCase. Properties were used when they seemed appropriate. Enum members defined by CSSAlign, CSSConstants, CSSDirection, CSSFlexDirection, CSSJustify, CSSPositionType, and CSSWrap have been converted to PascalCase. `CSharpTranspiler.js` will take care of the case conversion for the generated code. ### Method forwarders / duplication To keep the differences between the Java and C# transpiler minimal, I decided to leave some internal methods in camlCase. When appropriate, these methods were implemented as extension methods. ### Visual Studio Solution The solution's CSSLayout project is located in `src/csharp` and produces a portable class library Profile 259, which supports the .NET Framework 4.5, Windows 8+, Windows Phone 8.1, Windows Phone Silverlight 8, and Xamarin.Android, Xamarin.iOS, and Xamarin.iOS (Classic API). The solution also includes the test project, which uses NUnit. The code uses C# 6 syntax. ### File Format All *.cs files are encoded in UTF-8, LF line endings, no BOMs. *.sln, *.csproj, and *.config files are in UTF-8 with BOMs and CRLF line endings. ## File Notes ### CSSNode.cs - All methods are non-virtual so far. This is because I don't know which methods are intended to be virtual and which are not, so feedback is very appreciated here. For the projects I've done with the C# version, inheriting from CSSNode was not required. - The names of the properties for accessing the style width and height are named just `Width` and `Height` instead of `StyleWidth` and `StyleHeight`. All properties except the ones beginning with `Layout` are meant to be style related. - I've added Min/Max-Height/Width properties to CSSNode for reading and modifying CSSStyle fields because the `CSSNode.style` member is not accessible through CSSNode yet. - The measure function is defined as a .NET delegate. ### Spacing.cs This class seems not to be indented to be public, but gets exposed in CSSNode in the Java API through a field named `padding`. I decided to remove CSSNode.padding for now, because padding can already be modified with CSSNode's methods. ### CSSSpacingType.cs I did not liked the idea of using integers for the spacing type, so I've introduced an enum named CSSSpacingType, which is used in CSSNode for specifying the various types of spacing for paddings, margins, and borders. ## Build Script Only the transpiler is run from the gruntfile. However, there is a Makefile in the `src/csharp` directory that is used to build & test the solution and pack & upload the NuGet package. ## NuGet Package This pull request is based on an [unofficial adoption](https://github.com/pragmatrix/css-layout), which I am maintaining for some time. Also there is a [NuGet package available](https://www.nuget.org/packages/Facebook.CSSLayout/) that is named `Facebook.CSSLayout`. I am happy to transfer ownership, if this pull request is accepted. I've decided to properly clean up the API before creating this pull request, thereby introducing (hopefully very minor) incompatibilities with code written against the NuGet packages. And sorry, no history, I've never rebased the csharp version, but will from now on.
vjeux commented 2015-09-23 09:08:17 -07:00 (Migrated from github.com)

That's awesome, thanks for doing that work :)

That's awesome, thanks for doing that work :)
ColinEberhardt commented 2015-09-23 14:08:40 -07:00 (Migrated from github.com)

Very awesome @pragmatrix :-)

A few things that I am wondering about ..

  1. This should be added to the build - currently the grunt-based build lints, transpiles, runs the tests (in all languages), then packages the code for distribution. I'd love to see the C# code tested here also. This build is also part of the Travis-CI, which ensures Pull Requests do not break the code.
  2. It would also be great to have this added to the release process - my aim has been that tagged builds form a semantically versioning release of all the different transpiled versions of the code.

Of the above (1) should be quite easy. And for (2) I guess it would just require documentation. Can NuGet packages be released by a command line tool similar to the way npm works?

Very awesome @pragmatrix :-) A few things that I am wondering about .. 1. This should be added to the build - currently the grunt-based build lints, transpiles, runs the tests (in all languages), then packages the code for distribution. I'd love to see the C# code tested here also. This build is also part of the Travis-CI, which ensures Pull Requests do not break the code. 2. It would also be great to have this added to the [release process](https://github.com/facebook/css-layout/blob/master/dist/README.md) - my aim has been that tagged builds form a semantically versioning release of all the different transpiled versions of the code. Of the above (1) should be quite easy. And for (2) I guess it would just require documentation. Can NuGet packages be released by a command line tool similar to the way npm works?
pragmatrix commented 2015-09-24 00:34:43 -07:00 (Migrated from github.com)

Yes, nuget.exe push releases a nuget package given an api key was set up on the local machine with nuget.exe setapikey. Here is the line in the Makefile that releases the package.

Yes, `nuget.exe push` releases a nuget package given an api key was set up on the local machine with `nuget.exe setapikey`. [Here](https://github.com/facebook/css-layout/blob/master/src/csharp/Makefile#L21) is the line in the Makefile that releases the package.
woehrl01 commented 2015-09-25 00:32:51 -07:00 (Migrated from github.com)

thanks @pragmatrix !

do you mind changing the syntax to c# 5? Not everyone can use the latest VS.
E.g. I'm using this library (manual java port) in a .NET 4.0 VS2012 project so I have to compile the stuff by my self anyway. But changing the syntax everytime is a little bothering :)

I think this makes the library a little more re-usable.

thanks @pragmatrix ! do you mind changing the syntax to c# 5? Not everyone can use the latest VS. E.g. I'm using this library (manual java port) in a .NET 4.0 VS2012 project so I have to compile the stuff by my self anyway. But changing the syntax everytime is a little bothering :) I think this makes the library a little more re-usable.
pragmatrix commented 2015-09-25 01:02:37 -07:00 (Migrated from github.com)

@woehrl01 No I don't mind, Resharper got me there :) Can you do the pull request? And would a .NET 4.0 compatible NuGet package fit into your project?

@woehrl01 No I don't mind, Resharper got me there :) Can you do the pull request? And would a .NET 4.0 compatible NuGet package fit into your project?
pragmatrix commented 2015-09-25 02:01:40 -07:00 (Migrated from github.com)

@woehrl01 I had some time to see how hard it would be to get rid of using static, but since only a few constants were referenced from CSSLayout, the changes were minimal. I've prepared a branch here: https://github.com/pragmatrix/css-layout/tree/csharp5 . Please take a look if this compiles properly with VS2012.

@woehrl01 I had some time to see how hard it would be to get rid of `using static`, but since only a few constants were referenced from CSSLayout, the changes were minimal. I've prepared a branch here: https://github.com/pragmatrix/css-layout/tree/csharp5 . Please take a look if this compiles properly with VS2012.
woehrl01 commented 2015-09-25 02:17:40 -07:00 (Migrated from github.com)

@pragmatrix Awesome! Compiles perfectly, here. I just had to reference System to make this line work.

@pragmatrix Awesome! Compiles perfectly, here. I just had to reference `System` to make [this line](https://github.com/pragmatrix/css-layout/blob/csharp/src/csharp/Facebook.CSSLayout/Assertions.cs#L9) work.
Sign in to join this conversation.
No description provided.