Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Save function is misleading #105

Open
ViRuSTriNiTy opened this issue Mar 6, 2024 · 3 comments
Open

Save function is misleading #105

ViRuSTriNiTy opened this issue Mar 6, 2024 · 3 comments
Assignees
Labels
enhancement New feature or request SourceGenerator

Comments

@ViRuSTriNiTy
Copy link

ViRuSTriNiTy commented Mar 6, 2024

Hi there,

first things first, this project is fantastic. It is sooo much easier to use than writing your own source generator with all the tweaks needed to get it right. 👌

So, back to the actual issue I think is worth addressing. When I saw the save function showcased in some samples I thought, oh, really nice, now I don't need to emit, copy / move things around in csproj to get the generated files in the project. I took it to a test and generated some TypeScript code quick and efficently into the project. After a while I came back to generate some C# code in another project and all of a sudden no file was saved anymore. Same setup, almost same template code, except it was C# code. After almost giving up on it I decided to look at the source code and there I found a little detail that makes the Save function a "add source to compilation" function. So, you need to use a different extension to save C# code to a file. Bummer.

My current setup looks like this:

Foobar.cs.nt

# Do something and capture output

# Save the captured output, this will add the source code to the current compilation due to file extension .cs
Save output "Foobar.cs"

# Save it once again to create a temporary file in the project, transition from .cs.tmp to .cs will be done in .csproj
Save output "Foobar.cs.tmp"

Example.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <ItemGroup>
    <PackageReference Include="NTypewriter.SourceGenerator" />
  </ItemGroup>

  <!-- NTypewriter.SourceGenerator configuration -->
  <ItemGroup>
    <AdditionalFiles Include="Foobar.cs.nt" />
  </ItemGroup>

  <Target Name="CopyGeneratedFiles" AfterTargets="Build;Rebuild">
    <ItemGroup>
      <GeneratedFoobar Include="$(ProjectDir)Foobar.cs.tmp" />
    </ItemGroup>

    <Move SourceFiles="@(GeneratedFoobar)" DestinationFiles="$(ProjectDir)%(Filename)" />
  </Target>
  
  <ItemGroup>
    <Compile Remove="Foobar.cs" />
    <None Include="Foobar.cs" />
  </ItemGroup>

</Project>

Another option is to use EmitCompilerGeneratedFiles, but this can result in "path to long" issues and csproj configuration is also more complex.

How can we get rid of the second temporary save? I think something like adding a new function AddSource that does the same checks like Save will do and bring some clarity. Then I would need to call

AddSource
Save

and since Save would then actually write a file I can remove all that csproj magic from the project.

What do you think?

@NeVeSpl
Copy link
Owner

NeVeSpl commented Mar 6, 2024

That behaviour was introduced here #75.

In general, it is a good idea to split Save into two functions: Save and AddSource, but as you already noticed #106 adding *.cs files as a source has some problems right now.

@NeVeSpl NeVeSpl added SourceGenerator enhancement New feature or request labels Mar 8, 2024
@ViRuSTriNiTy
Copy link
Author

I have created a fork in hope to get the requires changes done. But it seems a split is only possible with heavy refactoring or introducing new APIs / deprecating old ones.

My approach would be to deprecate certain APIs and introduce more flexible ones. I think the steps would be

  • Define NTypewriter.Runtime.IGeneratedFile with properties Path and Content
  • Make IGeneratedFileReaderWriter.Write(string, string) obsolete
  • Add IGeneratedFileReaderWriter.Write(IGeneratedFile)
    • Allows GeneratedFileReaderWriter to cast for certain types implementing IGeneratedFile mentioned blow
  • Add AddSourceFunction based on SaveFunction
    • Pass NTypewriter.SourceGenerator.GeneratedCSharpFile implementing IGeneratedFile to AddRenderedItem
  • Implement IGeneratedFile in RenderedItem
  • Add some logic in ``GeneratedFileReaderWriterfor handlingRenderedItem` and `GeneratedCSharpFile` differently

What do you think?

@NeVeSpl
Copy link
Owner

NeVeSpl commented Mar 14, 2024

I would prefer to add a flag AddToCompilation on NTypewriter.RenderedItem instead of using IGeneratedFile as a marking interface, anyway NTypewriter does not have a reference on NTypewriter.Runtime, so using interface there defined is not an option. Passing that flag through the whole execution path should be enough without introducing any new types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request SourceGenerator
Projects
None yet
Development

No branches or pull requests

2 participants