bottlenoselabs / c2cs

Generate C# bindings from a C header.
MIT License
250 stars 18 forks source link

Reduce noise of changes of generated C# in Git log #90

Closed lithiumtoast closed 1 year ago

lithiumtoast commented 2 years ago

Auto-generated bindings can be quite large as seen here: https://github.com/flecs-hub/flecs-cs/pull/5. I would like to explore how to reduce the Git diff noise.

RandyPasion commented 2 years ago

I think the first key point in resolving the noise is a more controlled ordering when writing the C# code. Currently, regenerating the bindings, even with no changes to the source files, can result in different order of items in the output cs.

One easy(?) approach to this would be sorting by name. While that would help to reduce the diff noise, I think it has other shortcomings. Primarily that by-name doesn't really offer any cognitive organization to the code. Entirely unrelated items end up next to one another solely based on name.

A better approach, I think, would be sorting by source file and line. The source files should (if any hope remains for humanity) be deliberately organized with intent from the author. Sorting the output cs in the same order would help to preserve the original intent as well as consistency that reduces diff noise.

The second approach would also benefit from the group headers into regions improvement. (Or even separate cs files).

lithiumtoast commented 2 years ago

Currently, regenerating the bindings, even with no changes to the source files, can result in different order of items in the output cs.

This is interesting; I have not encountered this before.

One easy(?) approach to this would be sorting by name.

You mean globally? The functions should already be sorted by name but then there are things before and after all the functions with different names.

A better approach, I think, would be sorting by source file and line.

I agree. I would think of sorting by absolute file path of the header file, then by line, then by C# name.

Yes, #30, is something I was thinking of getting around to which could compliment this.

What do you think of having multiple .cs files?

RandyPasion commented 2 years ago

This is interesting; I have not encountered this before.

I thought so too. I noticed it while testing a small header for bindgen friendliness, so the differences were pretty apparent.

The functions should already be sorted by name but then there are things before and after all the functions with different names

This doesn't seem to the behavior currently, unless I'm misunderstanding. Even looking at the flecs cs file, it does not seem to follow by-name sorting.

I would think of sorting by absolute file path of the header file, then by line, then by C# name.

Not sure about absolute file path. Reason being, when a team where multiple members may be regenerating bindings, the absolute path of the file will differ from person to person depending on their os and where they store their repos locally - resulting in reordering. I would think just header file name would be sufficient.

What do you think of having multiple .cs files?

Separate regions or multiple cs files both effectively accomplish the same thing. I don't think either provides significant benefit over the other. Perhaps multiple cs files are more friendly to read or spot changes in the git diff since they are smaller, but it's mostly just personal preferences. Having one or the other would be a good improvement. Having the option to choose either is just icing on the cake.

lithiumtoast commented 2 years ago

The last dependabot GitHub actions workflow for flecs-cs seems to be a success with reducing noise of Git log changes!

https://github.com/flecs-hub/flecs-cs/pull/89/commits/1a89ac3f9223a434dfafeb4b7f18471036e84a76

I'm going to close this for now and leave the https://github.com/bottlenoselabs/c2cs/issues/30 open.

mikoro commented 1 year ago

I've been using the 4.0.8 version of the tool and enums are still shifting around everytime one generates the output file.

lithiumtoast commented 1 year ago

Thanks @mikoro, do you have an example by chance? Is it just enums?

mikoro commented 1 year ago

The source file location could be the reason:

        [CNode(Kind = "EnumConstant", Platform = "x86_64-pc-windows-msvc", Location = "xxx.h:8:1")]
        public const int XXX1 = 0;

        [CNode(Kind = "EnumConstant", Platform = "x86_64-pc-windows-msvc", Location = "xxx.h:8:1")]
        public const int XXX2 = 3;

        [CNode(Kind = "EnumConstant", Platform = "x86_64-pc-windows-msvc", Location = "xxx.h:8:1")]
        public const int XXX3 = 2;

        [CNode(Kind = "EnumConstant", Platform = "x86_64-pc-windows-msvc", Location = "xxx.h:8:1")]
        public const int XXX4 = 1;

The c source:

enum {
    XXX1= 0,
    XXX2 = 1,
    XXX3 = 2,
    XXX4 = 3,
};

I have IsEnabledEnumsDangling = true.

lithiumtoast commented 1 year ago

Thanks I'll check this out this weekend.

lithiumtoast commented 1 year ago

@mikoro Should be fixed via https://github.com/bottlenoselabs/c2cs/pull/125/commits/94c8b994da08f01b2bf91224305a7f1ad3614d38. Let me know if not, and I can reopen.