[Designer discussion] Make InitializeComponent code-gen more configurable

[KlausLoeffelmann]

Hey Community,

we would love your take on this issue, which we have been discussing internally.
From some comments and issues from the Community feedback channel, we feel it might be a good idea, to make the code-gen for IntializeComponent more configurable, especially around automatic delegate method group conversion when hooking up events in C# and the shortening of type names in general.

While we had positive feedback in general, there were some dev teams using WinForms, which had problems, when they were sharing UI code between Framework and .NET and In-Proc and Out-Of-Proc Designer. In this case, the Forms/UserControls would basically roundtrip, but the different type-name serialization was annoying. In the case of using the delegate conversion, the In-Proc designer cannot even process that code-style - it would lead to code loss.

New style:

_okButton = new Button(); // We could even do _okButton = new();
_okButton.Click += OkButton_Click;

Old Style:

this._okButton = New System.Windows.Forms.Button();
this._okButton.Click += new System.EventHandler(this.OkButton_Click);

So, the issue to discuss is: Should we introduce additional configuration options or should we hard code to omit the delegate conversion per se and live with the differences between out-of-proc and in-proc designer, which just would lead to different coding-styles, but would round-trip OK (Everything would work out of the box).

Here is the internal Designer-Repo PR description:

Currently, when we generate Visual Studio and C# InitializeComponent Code in the Out-Of-Proc Designer, we are taking .editorConfig settings for a couple of code item styles into account:

• We honor the indention settings.
• We honor the usage of this or Me (or rather their omission, where it's ok in the context).

But we have other areas, though, where we're applying a certain code style without asking.

• Simplifying type names where it's possible (replacing fully qualified names by short type names)
• Simplifying event hook-ups by using delegate method group conversions. This one can become especially problematic, since it does no longer allow a Form to roundtrip in the in-proc- and the out-of-proc Designer. Meaning: After you serialized a Form with the Out-Of-Proc-Designer, it won't be able to be deserialized in the In-Proc Designer: The In-Proc Designer reads the existing code based on CodeModel (and not based on Roslyn), and it does not understand delegate method group conversions. What's especially bad in those scenarios: The Designer does not throw in this case - it just swallows the code and deletes the event hook-ups.

This PR is an approach to expand the configuration option for the latter.
It introduces the Project settings:

ApplicationCodeGenMethodGroupConversions: True/False
ApplicationCodeGenSimplifiedTypeNames: True/False

It also introduces the necessary code changes, to actually honor the option to generate the instantiation of the Event Handlers, when the respective option is set to false.

The reason we cannot use .editorConfig for this, are:

a) There isn't really a respective setting, which would make sense to use.
b) We cannot extend the schema of the .editorConfig in a meaning full way,
c) It wouldn't also not make sense conceptionally, since those changes folks most likely would like to address ONLY in the context of InitializeComponent to be backwards compatible with the In-Proc-Designer. They don't want to have those coding styles for analyzers or code fixers. Also, it makes sense to just do it on project level, because it's a project which would be most likely contain Forms, which need to be shared (for example during a project migration) for a longer period of time (or even forever).

[tbolon]

Should really the code-gen serializer be so configurable? How about a simple legacy/modern switch?

I thing the problematic is only about being compatible with the inproc vs outofproc designer. Maybe in the future you want to change some serialization code to use more recent c# features and any shared code would need to disable this specific code-style in the csproj. Why not having a single ApplicationCodeGenLegacyStyle: True/False (false by default).

Even more: why not setting this setting to true implicitely as soon as the the project <TargetFrameworks> target both net48 AND net5+ ?

[KlausLoeffelmann]

Why not having a single ApplicationCodeGenLegacyStyle: True/False (false by default). Even more: why not setting this setting to true implicitely as soon as the the project target both net48 AND net5+ ?

Brilliant approach! That sounds like the best way to go, but I would still have that with an option. Here - what do you think? (Also @merriemcgaw)

• Out-Of-Proc Framework -> Legacy Formatting (Default)
• Multi-Targeting Framework/.NET -> Legacy Formatting (Default)
• True|False

The last option would only make sense for Multi-Targeting or Out-Of-Proc Framework, and ignored otherwise, meaning: For .NET it will always be optimized. So, you could force modern code gen for Framework or mixed, but you would do legacy by default.

I guess that would also address the concern of my colleague @Shyam-Gupta, who pointed out that we should not overkill with configuration options. And now, I have to say, I am convinced. Your guys' approach is better.

[paul1956]

Following editorConfig where possible is a great idea, it would stop the thrash in GitHub with every other edit and reformat. Using a single configuration switch to handle projects that need to go between .Net and Framework seems like a better approach especially since not everything would be supported by editorConfig.

[merriemcgaw]

Makes a lot of sense to me. Great ideas @KlausLoeffelmann and @paul1956

[RussKie]

If you decide to go down this route, please consider

• Serialize layout in lexicographical order inspired by Lexicographical order designer code #3652 and, to a degree, by A variety of accessibility-related fixes for the contents of the main pane on the Config page  gitextensions/gitextensions#8682 (comment).
• Serialize Controls.Add() abiding by tab order settings inspired by A variety of accessibility-related fixes for the contents of the main pane on the Config page  gitextensions/gitextensions#8682 (comment), and is aimed to update the designer serialization to
  • use the "natural" tab order,
  • reorder container.Controls.Add(control) statements according to the programmatic tab order, if it is set.

[KlausLoeffelmann]

I don't see how we can do the latter, without breaking dock layouts and dedicated overlapping controls.
The order in which controls get added determines the Z-Order. And the Z-Order is nothing I would change in a black-box setting. That's a BIG breaking change.

[RussKie]

That's not entirely correct. There are limitations, yes, the docked controls can't be re-ordered without breaking the layout; but all others - can. It's done here: https://github.com/microsoft/winforms-designer/pull/5122.