-
Notifications
You must be signed in to change notification settings - Fork 20
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
Replace T4 with raw string literale #29
Conversation
29e1cb3
to
79da641
Compare
7bd6526
to
2a6142d
Compare
{ | ||
return value.value; | ||
} | ||
|
||
public static explicit operator A(int value) | ||
public static implicit operator A(int value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what the reason of different result, implicit and explicit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh.. simply my mistake... fixed. 0635db9
using System; | ||
|
||
namespace FileGenerate | ||
#pragma warning disable CS8669 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is // <auto-generated>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in 7aca1e1
|
||
namespace FileGenerate | ||
#pragma warning disable CS8669 | ||
using System;namespace FileGenerate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
require new line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed..!
{ | ||
[System.ComponentModel.TypeConverter(typeof(ATypeConverter))] | ||
public readonly partial struct A : IEquatable<A> | ||
readonly partial struct A : IEquatable<A> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The master branch seems to omit public
modifier as well.
https://github.com/Cysharp/UnitGenerator/blob/master/src/UnitGenerator/CodeTemplate.tt#L34
Probably, since it is partial, the handwritten code will be followed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, omit modifier is ok.
@@ -57,7 +51,7 @@ public override int GetHashCode() | |||
|
|||
public override string ToString() | |||
{ | |||
return "A(" + value + ")"; | |||
return value.ToString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
different ToString result?(I don't know which is better)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I generate again with master
branch, I seem to get a diff of this file.
It looks like value.ToString()
is probably the implementation currently in use. Is this correct?
https://github.com/Cysharp/UnitGenerator/blob/master/src/UnitGenerator/CodeTemplate.tt#L88
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recall changing it to a straightforward ToString because having type information made it harder to read, and it also reduced the convenience of ToString.
So, this implementation is OK!
Currently, T4 is used to implement code generation, but it will be replaced with a rustic template.
motivation: