canhorn / EventHorizon.Blazor.TypeScript.Interop.Generator

This project is a Blazor Interop C# Generator, has a sample against the BabylonJS library.
https://wonderful-pond-05f7b3b10.azurestaticapps.net/
MIT License
132 stars 19 forks source link

Fix added reserved keywords to DotNetNormalizer.cs #59

Closed badcommandorfilename closed 2 years ago

badcommandorfilename commented 2 years ago

Added string and this to C# reserved keyword list

Hi, I was using this library with tensorflow.js and had to add these keywords to work with the JS names used in that library.

This is pretty low-hanging fruit, so I'm happy to extend this with more of the reserved names in C# if you're looking for help with contributions.

canhorn commented 2 years ago

@badcommandorfilename Thanks for adding these normalization keywords! Can you add to the scenario testing, the two files are below that test this for Accessors and Properties: https://github.com/canhorn/EventHorizon.Blazor.TypeScript.Interop.Generator/blob/05dedfdd7469b2ada3fc210df7d8496f53fe82ef/Tests/EventHorizon.Blazor.TypeScript.Interop.Generator.Tests/GenerateClassStatementStringTests/Accessors/Scenarios/DotNetNormalized.ts https://github.com/canhorn/EventHorizon.Blazor.TypeScript.Interop.Generator/blob/05dedfdd7469b2ada3fc210df7d8496f53fe82ef/Tests/EventHorizon.Blazor.TypeScript.Interop.Generator.Tests/GenerateClassStatementStringTests/Properties/Scenarios/DotNetNormalized.ts

You will have to update the corresponding DotNetNormalized.Expected.txt that will be next to the files. These are the source code from TypeScript and the generated C# that is expected to be generated.

badcommandorfilename commented 2 years ago

@canhorn I've updated the tests with basic coverage - I found another one at https://github.com/canhorn/EventHorizon.Blazor.TypeScript.Interop.Generator/blob/05dedfdd7469b2ada3fc210df7d8496f53fe82ef/Tests/EventHorizon.Blazor.TypeScript.Interop.Generator.Tests/GenerateClassStatementStringTests/Constructors/DotNetNormalizedArguments.ts

I'm not sure if you prefer to squash commits before merging, so let me know if there are any more changes.

codecov[bot] commented 2 years ago

Codecov Report

Merging #59 (d41dab8) into main (05dedfd) will increase coverage by 0.00%. The diff coverage is 100.00%.

:exclamation: Current head d41dab8 differs from pull request most recent head feaeb4f. Consider uploading reports for the commit feaeb4f to get more accurate results Impacted file tree graph

@@           Coverage Diff           @@
##             main      #59   +/-   ##
=======================================
  Coverage   98.85%   98.85%           
=======================================
  Files          92       92           
  Lines        4275     4278    +3     
  Branches      384      384           
=======================================
+ Hits         4226     4229    +3     
  Misses         18       18           
  Partials       31       31           
Impacted Files Coverage Δ
....Interop.Generator/Normalizers/DotNetNormalizer.cs 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 05dedfd...feaeb4f. Read the comment docs.

canhorn commented 2 years ago

@badcommandorfilename Look Good! The checks failed, but that was because of missing secrets, that is expected. I will fix that in the future so it will not run those tasks when its a external PR.

Also I noticed that the author on some of the commits are under another name. If you squash the commit under your preferred name that should fix the mixed authors. But it is up to you.

I am fine to merge this PR either way. Let me know. :)

badcommandorfilename commented 2 years ago

Thanks for spotting the author name @canhorn - I've fixed it up now and I'm happy to merge.

This is a very cool project, by the way! Thanks for making something like this. I'm very happy to contribute any more fixes or features as I keep using it.

canhorn commented 2 years ago

Thanks! Anymore contributions are welcomed!