Conversation
Merge branch 'master'
Merge branch 'master'
merge from dev
merge from master
|
Linking the correct issue: fsharp/fslang-suggestions#364 |
|
you guys should add some tests ;-) |
|
yeah we will work on it therefore its WIP thanks @forki |
|
awesome |
|
thanks @dsyme this also fix printing () as null |
src/utils/sformat.fs
Outdated
|
|
||
| if FSharpType.IsTuple reprty then | ||
| TupleValue (FSharpValue.GetTupleFields obj |> Array.toList) | ||
| let tyArgs = reprty.GetGenericArguments() |
There was a problem hiding this comment.
Why not use Reflection.FSharpTYpe.GetTupleElements()?
There was a problem hiding this comment.
Thanks didn't notice that this function exiat
There was a problem hiding this comment.
using it without the Reflection namespace solvie this
|
@AviAvni Please check behaviour with the |
|
@dsyme the option type also use this attribute so when I'll change the logic to check this also option will work |
|
@dsyme I changed the logic like you asked |
src/utils/sformat.fs
Outdated
| (attr.Flags &&& CompilationRepresentationFlags.UseNullAsTrueValue) = CompilationRepresentationFlags.UseNullAsTrueValue | ||
| | _ -> false | ||
| if isNullaryUnion then | ||
| let nullaryCase = FSharpType.GetUnionCases typ |> Array.item 0 |
There was a problem hiding this comment.
I'm not certain the nullary union case has to come first (I'd need to check), and in any case we might lift that restriction in the future. Better to search for the nullary case explicitly.
There was a problem hiding this comment.
I change it to find the union case with 0 fields
|
The CI failure is just AppVeyor, probably a timeout. |
|
@KevinRansom I'm happy with this. |
Fixing option null printing for record values (WIP)
Added more printing for tests for records
merge from master
|
@dsyme all green now |
|
Thank you for this contribution, it's great to have this fixed after all these years |
See fsharp/fslang-suggestions#364
I'm working with https://github.com/nelak on this as part of mentorship program
please review