Skip to content

Take the namespace of generated files from the RootNamespace property#50

Merged
StefH merged 0 commit intoStefH:mainfrom
justinwritescode:main
Jan 15, 2023
Merged

Take the namespace of generated files from the RootNamespace property#50
StefH merged 0 commit intoStefH:mainfrom
justinwritescode:main

Conversation

@justinwritescode
Copy link
Copy Markdown

Change to take the namespace from the RootNamespace property instead of from the AssemblyName. This makes the generated code configurable.

@StefH
Copy link
Copy Markdown
Owner

StefH commented Oct 11, 2022

@justinwritescode
Did you use a different newline for the files? Because it seems that all lines are changed?

@justinwritescode
Copy link
Copy Markdown
Author

I wrote it on a Mac....sorry....

@justinwritescode
Copy link
Copy Markdown
Author

Ah...yes...it must be the line endings. When I click "Hide whitespace changes" it shows up fine. Sorry about that....

@StefH
Copy link
Copy Markdown
Owner

StefH commented Oct 11, 2022

Can you edit this on a windows pc ? Or change the line-endings to CRLF?

@justinwritescode
Copy link
Copy Markdown
Author

Actually, it looks like your line endings are inconsistent. Have a look at IFileGenerator.cs. On the left is the repo code, right is a version I just opened and saved with CRLF endings.

image

And here's FluentBuilderClassesGenerator.IDictionary.cs:

image

And here's Header.cs:

image

I checked some of the others I didn't touch and it looks like your files pretty much alternate CRLF and LF line endings....

Anyway, these all have consistent CRLF line endings now. If it's too confusing for you to look at, you can click the gear icon in the diff viewer and click "Hide whitespace" as shown below.

image

Cheers! And thanks for a great lib!

@StefH
Copy link
Copy Markdown
Owner

StefH commented Oct 11, 2022

I will take a look tomorrow.

Thank you for investigating.

Copy link
Copy Markdown
Owner

@StefH StefH left a comment

Choose a reason for hiding this comment

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

  1. Can you write some comments in your PR on how that "build_property.RootNamespace" should be used? (Then I will copy to this to the readme)
  2. And can you add a unit-test for this change? (Maybe you need a new test-project for this?)

{
/// <see cref="Compilation.AssemblyName"/>
public string AssemblyName { get; }
public string NamespaceName { get; }
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can you add some comment for this new property?

@StefH
Copy link
Copy Markdown
Owner

StefH commented Oct 14, 2022

@justinwritescode
Did you have time yet to check my review comments?

@justinwritescode
Copy link
Copy Markdown
Author

Yes but I haven't had time to implement yet...

@StefH StefH merged commit d3c90eb into StefH:main Jan 15, 2023
@StefH StefH added the enhancement New feature or request label Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants