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

chore(csharp): refactor writer, ast node, and code block into commons #4578

Merged
merged 4 commits into from
Sep 8, 2024

Conversation

dsinghvi
Copy link
Member

@dsinghvi dsinghvi commented Sep 7, 2024

In this PR, we update the generator commons package to export an AbstractWriter, AbstractAstNode and CodeBlock. These core building blocks will help hit the ground running as new generators are added.

@dsinghvi dsinghvi requested review from dcb6 and amckinney September 7, 2024 23:09
@dsinghvi dsinghvi merged commit bc89230 into main Sep 8, 2024
39 checks passed
@dsinghvi dsinghvi deleted the dsinghvi/setup-commons-writer branch September 8, 2024 17:32
*/
public writeNodeStatement(node: AbstractAstNode): void {
node.write(this);
this.write(";");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Details like this might need to be left out of the abstract classes - not every language ends statements with a ;. I know most languages do use this syntax though, so we have a couple options:

  1. Leave this as-is and rely on method overrides in language-specific writers.
  2. Refactor the AbstractWriter to be fairly similar to the original generators/commons.Writer.

You might have already had (1) as the desired solution, but figured it was worth calling out.

* Writes text but then suffixes with a `;`
* @param node
*/
public controlFlow(prefix: string, statement: AbstractAstNode): void {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to above - control flow and blocks will differ based on the language, so the decision applies here too.

@@ -5,7 +5,7 @@ import { FernGeneratorCli } from "@fern-fern/generator-cli-sdk";
import { readFile } from "fs/promises";
import yaml from "js-yaml";
import path from "path";
import { ReferenceConfigBuilder } from "./ReferenceConfigBuilder";
import { ReferenceConfigBuilder } from "./reference";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants