Fixes #3171. Remove View and subclass constructors with parameters.

[BDisp]

Fixes

• Fixes Remove View and subclass constructors with parameters #3171

Proposed Changes/Todos

• View
• Button
• CheckBox
• ComboBox
• FrameView
• Label
• ListView
• ScrollBarView
• ScrollView
• ContextMenu
• Dialog
• OpenDialog
• SaveDialog
• FileDialog - Parameter can't be removed because there isn't a public _fileSystem field and it's only for testing.
• TextField
• TimeField
• Toplevel
• Window
• RadioGroup
• TextView
• MenuBar
• Menu
• TextValidateField

Changes checklist:

• Rename Initialize to SetInitialProperties
• Remove namespace braces
• Prefix private fields with underscore
• Remove unused using
• Remove parenthesis on objects initializers

Pull Request checklist:

• I've named my PR in the form of "Fixes #issue. Terse description."
• My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
• My code follows the Terminal.Gui library design guidelines
• I ran dotnet test before commit
• I have made corresponding changes to the API documentation (using /// style comments)
• My changes generate no new warnings
• I have checked my code and corrected any poor grammar or misspellings
• I conducted basic QA to assure all features are working

[BDisp]

@tig this is ready for review. Thanks.

[tig]

See comments.

FWIW, I don't think we have to remove them ALL at once, but I DO think that before we release V2 ALL View constructors with parameters should be gone unless there's a really, really good reason for it.

[BDisp]

FWIW, I don't think we have to remove them ALL at once, but I DO think that before we release V2 ALL View constructors with parameters should be gone unless there's a really, really good reason for it.

I thought you only want get rid those with frame parameters. Now it's more clear and only the readonly properties that only be set through a parameter passed on the constructor will be used. I'll procced then.

[tig]

FWIW, I don't think we have to remove them ALL at once, but I DO think that before we release V2 ALL View constructors with parameters should be gone unless there's a really, really good reason for it.

I thought you only want get rid those with frame parameters. Now it's more clear and only the readonly properties that only be set through a parameter passed on the constructor will be used. I'll procced then.

For "readonly" parameters, init or priviate init can be used.

[BDisp]

@tig this is finally already ready for review. The only I didn't removed was the FileDialog - Parameter can't be removed because there isn't a public getter for _fileSystem field. I don't know if it's intentional not exposing it or I can create a FileSystem property with a init setter.

[tznind]

I don't know if it's intentional not exposing it

There is a public constructor:

public FileDialog () : this (new FileSystem ()) { }

So you could make this one internal:

public FileDialog (IFileSystem fileSystem)

or I can create a FileSystem property with a init setter.

Need to be careful here as the file system prop is shared with the dialog style. init could work but just be a bit careful and ensure there doesn't end up a de-sync between dialog and style classes.

[BDisp]

Need to be careful here as the file system prop is shared with the dialog style. init could work but just be a bit careful and ensure there doesn't end up a de-sync between dialog and style classes.

My question was whether I could give the user access to obtain data from the FileSystem. If there is no problem with this problem then I will use an init setter. It will practically work the same, just giving the user visibility of the FileSystem data. Thanks.

[tig]

@BDisp did you apply resharper code-cleanup to all the files in the solution in this PR?

For example, it looks like PosDim.cs has been cleaned up, but you didn't make any functional changes.

I just want to make sure I know what I'm looking at before I dive in and review.

[BDisp]

@BDisp did you apply resharper code-cleanup to all the files in the solution in this PR?

Yes I did for all the files that were touched by the changes.

For example, it looks like PosDim.cs has been cleaned up, but you didn't make any functional changes.

It changed because I did a find/replace on the solution scope from TextField () { to TextField {. and it changed the line 153 which is now 163.

I just want to make sure I know what I'm looking at before I dive in and review.

I understand.

[BDisp]

I don't know if it's intentional not exposing it

There is a public constructor:

public FileDialog () : this (new FileSystem ()) { }

So you could make this one internal:

public FileDialog (IFileSystem fileSystem)

or I can create a FileSystem property with a init setter.

Need to be careful here as the file system prop is shared with the dialog style. init could work but just be a bit careful and ensure there doesn't end up a de-sync between dialog and style classes.

As in the comment is explicitly is for testing ( /// <remarks>This overload is mainly useful for testing.</remarks>) I'll change it to internal as recommended by the @tznind. Thanks.

[BDisp]

@tig I remembered that I only started running resharper code-cleanup after you asked. Now I reflected that it might not have been done for some files that were previously changed and that perhaps it was not changed in subsequent commitments. After all, there were still 46 files to be carried out by resharper code-cleanup and already corrected in the commit b4650fa.

[BDisp]

Why R# Ctrl-E, C convert simples auto-properties to backed fields? Like the following.

From:
public object Data { get; set; }

To:

object _data;

public object Data {
	get => _data;
	set => _data = value;
}

Is this really necessary?

[BDisp]

Why R# Ctrl-E, C convert simples auto-properties to backed fields? Like the following.

From: public object Data { get; set; }

To:

object _data;

public object Data {
	get => _data;
	set => _data = value;
}

Is this really necessary?

I reverted this changes and reapply the R# cleanup code again and this doesn't happened again. I don't know why this conversion was apply before. Solved.

[tig]

I think I'll have some bandwidth this week to work on this again.

[tig]

@BDisp please revert the TextFormatter changes you made in this PR.

If you can, also please, revert any code reformatting you did that was not in a file impacted by actually removing the parameters from constructors.

This will greatly assist me in merging this. Thanks.

[BDisp]

@BDisp please revert the TextFormatter changes you made in this PR.

I will revert it.

If you can, also please, revert any code reformatting you did that was not in a file impacted by actually removing the parameters from constructors.

This will greatly assist me in merging this. Thanks.

Unfortunately this won't be possible because they was done all together with the removing parameters from constructors and it isn't easy to revert that, sorry. My brain explode by only thinking to do that 🤯

[tig]

This has been merged into #3202.

However, #3202 is not yet ready because merging this PR into it broke a bunch of stuff I had working.

I'm closing.

[BDisp]

Sorry @tig. I forgot to revert the TextFormatter changes.