Terminal.Gui icon indicating copy to clipboard operation
Terminal.Gui copied to clipboard

Use of `Data` property

Open tznind opened this issue 2 years ago • 6 comments

Describe the bug Just noticed that there are some internal uses within Terminal.Gui library of the Data field e.g.

https://github.com/gui-cs/Terminal.Gui/blob/fb3dde2289a9929434f98d2cd7649389312585fe/Terminal.Gui/Views/FrameView.cs#L55

https://github.com/gui-cs/Terminal.Gui/blob/fb3dde2289a9929434f98d2cd7649389312585fe/Terminal.Gui/Views/Wizard/WizardStep.cs#L42

The documentation for this property indicates that we won't do this:

/// <summary>
/// Gets or sets arbitrary data for the view.
/// </summary>
/// <remarks>This property is not used internally.</remarks>

Within the designer, I use Data to track the field member name in the source code written out to .Designer.cs. And to store the Design (editable properties blueprint).

So there is a meaning when Data has a value (user designed this view) and theres a meaning when it is null (this view is an artifact of another view e.g. ContentView).

There may be other library users making similar assumptions.

Is it please possible to move the internal uses of this property to a new field or adjust the approach?

tznind avatar May 27 '23 09:05 tznind

Here my opinion on this. View class has a Data property which is for the user use it as he like. We should avoid use this property internally and only use it on app, like the scenarios, etc. @tznind you said use Data property and I ask if it's possible a user set the Data property for whatever he want, at least at design time, I guess. So, my suggestion is use the Id property to handle with field member name. Is this isn't enough then probably you have to create a sub-file to track that. e.g. app.cs, app.Designer.cs, app without extension or with an extension properly named for track the field members. But I may be wrong.

BDisp avatar May 27 '23 13:05 BDisp

I believe it is possible, and we should remove the usages of Data internally, except if used for a view that is never exposed eternally.

But, to be clear, Designer is NOT part of the library and should be able to use it as it see fits.

tig avatar May 28 '23 04:05 tig

Related: https://github.com/gui-cs/Terminal.Gui/issues/2502

This PR (removing TopLevel completely) will fix one egregious violation of us using Data internally.

tig avatar May 28 '23 04:05 tig

We should add a Unit Test that uses reflection to find every subclass of View and test and Assert.Null (view.Data) after construction and after IsInitialized = true.

tig avatar May 28 '23 04:05 tig

But, to be clear, Designer is NOT part of the library and should be able to use it as it see fits.

Yes, I know. I only was asking @tznind if the Data was available to user to do what he want after compile or contains some track that is used during the runtime avoiding the user really use the Data property. If it's only used during the design and not after compiled, there no problem with that.

BDisp avatar May 28 '23 08:05 BDisp

Audit of Data usage outside ./Views (core Terminal.Gui):

  1. Border.Arrangment.cs Usage: Sets and reads Data on Button instances to store a ViewArrangement value. Example: button.Data = arrangement; Arranging = (ViewArrangement)(Focused?.Data ?? ViewArrangement.Fixed); Purpose: Used for internal arrangement logic in the adornment system. Conflict: This is a core/internal usage of Data (not just in public-facing Views).
  2. View.Keyboard.cs Usage: Sets Data = context (likely for keyboard context passing). Purpose: Used for internal event/context logic. Conflict: Internal/core usage.
  3. View.cs Definition: Declares the public property: public object? Data { get; set; } Documentation states: "Gets or sets arbitrary data for the view." "This property is not used internally." Conflict: The above files do use it internally, violating the documentation.
  4. KeyBinding.cs Usage: Sets Data = context and Data = data in constructors and methods. Purpose: Used for context passing in key bindings. Conflict: Internal/core usage.
  5. IInputBinding.cs, MouseBinding.cs Definition: Declares public Data property for bindings. Usage: No clear internal/core usage detected, but could be used for context. Summary: There are several instances in the core of Terminal.Gui (outside ./Views) where the Data property is used for internal logic, especially in adornment, keyboard, and binding systems. This directly conflicts with the documentation stating that Data is not used internally.

Recommendation:

Refactor these internal usages to use a new internal/private field or property (e.g., _internalData or InternalArrangement, etc.). Update documentation to accurately reflect usage, or ensure Data is truly not used internally. Would you like to proceed with refactoring these internal usages?

tig avatar Oct 03 '25 18:10 tig