Paste containers

[tznind]

Support for copying and pasting containers.

Fixes #143

[tznind]

@BDisp I am seeing this exception sometimes when pasting TabView or clicking around it's border. Its a null reference in SetHasFocus. Any ideas? has this been fixed in main or is it something I am doing wrong in the designer do you think?

Maybe its because I am leaving focus something that no longer is attached to Application.Top do you think? (i.e. becuase it was removed/deleted).

[BDisp]

Maybe its because I am leaving focus something that no longer is attached to Application.Top do you think? (i.e. becuase it was removed/deleted).

I think so, when you do this toReturn.RemoveAll(children.Contains); will dispose be running to clean all the references? It might be something that is needed to fix in the Terminal.Gui, but if you can write a unit test that can reproduce this will be very useful. When you are pasting do you are creating a new type with the same characteristics or the references?

[tznind]

When you are pasting do you are creating a new type with the same characteristics or the references?

Process is

• Look at type to copy
• Ask ViewFactory to create a new instance of that Type
• Add copy to destination container using AddViewOperation (so it gets a Design and a unique new fieldname)
• Cycle all designable properties on the original and apply them to the desination with Property.SetValue(...) which is basically by reference.

Now if copying a container after pasting the container itself it recursively goes through the sub-views/containers creating instances of them too.

But for TabView its a bit complicated because ViewFactory creates an instance with 2 demo tabs and because AddViewOperation adds the view you specify to the currently selected tab.

So the process is

• Remove all tabs from clone
• Foreach tab in original
  • Create a new tab
  • Select that tab in original and clone
  • Copy and paste the contents

I'll dig more see if I can get a minimum repro of when it happens and then see if I can write a unit test.

[BDisp]

You can test by adding the Terminal.Gui project instead the nuget package and check what object isn't set to an instance of an object.

[tznind]

So I checked out main and attached debugger to the running TerminalGuiDesigner process. And this is where the null comes from:

Null reference in void SetHasFocus (bool value, View view, bool force = false)

So the OnLeave handler is clearing the focused variable.

[BDisp]

So the OnLeave handler is clearing the focused variable.

Ah that is. After return from the OnLeave it must check again the focused variable if it's null. If you have a unit test I'll use it, otherwise I'll simulate one.

[tznind]

It seems there are several ways this can happen. Here is MenuBar clearing the focused property as a side effect of CleanUp calling CanFocus setter.

[tznind]

Ah that is. After return from the OnLeave it must check again the focused variable if it's null. If you have a unit test I'll use it, otherwise I'll simulate one.

I don't have a unit test so happy if you can create one, thanks! ❤️

[BDisp]

@tznind try move to inside the first if statement like bellow and check if it throws, please.

        void SetHasFocus (bool value, View view, bool force = false)
        {
            if (hasFocus != value || force) {
                hasFocus = value;
                if (value) {
                    OnEnter (view);
                } else {
                    OnLeave (view);
                }
                SetNeedsDisplay ();

                // Remove focus down the chain of subviews if focus is removed
                if (!value && focused != null) {
                    focused.OnLeave (view);
                    focused.SetHasFocus (false, view);
                    focused = null;
                }
            }
        }

[tznind]

What about just copying to a local variable e.g.:

// Remove focus down the chain of subviews if focus is removed
    if (!value && focused != null) {
+       var f = focused;
+       f.OnLeave (view);
+       f.SetHasFocus (false, view);
-       focused.OnLeave (view);
-       focused.SetHasFocus (false, view);
        focused = null;
    }

[BDisp]

It's a good solution too. What I ask is if this if statement needs to be running if the hasFocus value didn't changed at all. I already have a unit test that can get the error, check it please. Can I ask you if you can open this issue in the Terminal.Gui, please.

        [Fact, AutoInitShutdown]
        public void SetHasFocus_Do_Not_Throws_If_OnLeave_Remove_Focused_Changing_To_Null ()
        {
            var view1Leave = false;
            var subView1Leave = false;
            var subView1subView1Leave = false;
            var top = Application.Top;
            var view1 = new View { CanFocus = true };
            var subView1 = new View { CanFocus = true };
            var subView1subView1 = new View { CanFocus = true };
            view1.Leave += (e) => {
                view1Leave = true;
            };
            subView1.Leave += (e) => {
                subView1.Remove (subView1subView1);
                subView1Leave = true;
            };
            view1.Add (subView1);
            subView1subView1.Leave += (e) => {
                // This is never invoked
                subView1subView1Leave = true;
            };
            subView1.Add (subView1subView1);
            var view2 = new View { CanFocus = true };
            top.Add (view1, view2);
            Application.Begin (top);

            view2.SetFocus ();
            Assert.True (view1Leave);
            Assert.True (subView1Leave);
            Assert.False(subView1subView1Leave);
        }

[BDisp]

This is always false Assert.False(subView1subView1Leave); because the event will not be invoked.

[BDisp]

What I ask is if this if statement needs to be running if the hasFocus value didn't changed at all.

Unless other view than the focused wants to receive this event. I think I'll use your fix, what do you think?

Edit: My fix still throw the exception. For sure your fix is better :-) Please submit the PR with the unit test.

[tznind]

What I ask is if this if statement needs to be running if the hasFocus value didn't changed at all.

Unless other view than the focused wants to receive this event. I think I'll use your fix, what do you think?

Sounds good, the simpler the fix the less technical debt 😄

Ok I will submit.

[tznind]

Here is the fix https://github.com/gui-cs/Terminal.Gui/pull/2237