Running Application.Begin sometimes crashes on keybindings

[tznind]

Describe the bug
Occurs in TGD test setup step

Application.Begin(new Toplevel
{
    Width = Dim.Fill(),
    Height = Dim.Fill(),
});

Run TableViewTests in latest nuget package update PR

Message is:

Message: 
System.InvalidOperationException : A key binding for CursorUp exists (KeyBinding { Commands = Terminal.Gui.Command[], Scope = Application, BoundView = , Context =  }).

  Stack Trace: 
KeyBindings.Add(Key key, KeyBindingScope scope, Command[] commands)
Application.AddApplicationKeyBindings()
Application.InternalInit(ConsoleDriver driver, String driverName, Boolean calledViaRunT)
Tests.SetUp() line 22
InvokeStub_Tests.SetUp(Object, Object, IntPtr*)
MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

Not sure how to reproduce beyond running this test file. Its quite a simple test though.

Fix should be pretty simple, basically don't add the same keybinding twice just because Init/Begin is called again.

[tznind]

@tig / @BDisp if we could ideally fix this without merging anything else that are big changes then I can finally merge the TGD nuget update PR gui-cs/TerminalGuiDesigner#292

Currently what happens is I get down to small number of failing tests or behaviors and raise issue e.g. #3652 but the fix for that is +/- 800 lines and ships in a nuget package with many more other breaking changes.

This results in a loop, where the TGD nuget package update PR never reaches a mergeable state. I don't mind big changes but just need to find a way where the designer PRs can still reach merged state every now and then.

[BDisp]

I understand, I think that TG should also publish more nuget packages with only small changes to avoid too many breaking changes. This would avoid TGD being inactive for a long time and would gradually update, in a more controlled way, a new nuget package.

[BDisp]

I already ran this test using Application.Init (new FakeDriver () or Application.Driver = new FakeDriver (); and none of them fail.

    [Fact]
    public void Calling_Begin_After_Init_Does_Not_Throws_KeyBinding_Already_Exist ()
    {
        //Application.Init (new FakeDriver ());
        Application.Driver = new FakeDriver ();

        var exception = Record.Exception (() => Application.Begin (new ()
        {
            Width = Dim.Fill (),
            Height = Dim.Fill (),
        }));

        Assert.Null (exception);

        Application.Top!.Dispose ();
        Application.Shutdown ();
    }

[tig]

I think I know what this is. I'll try to look at it later today.

Re: nuget. Why not have TGD use -v2_develop packages that are published until alpha is released?

[BDisp]

@tig the v2_develop packages are hard to find. Can you help me find them, please. Thanks.

[tig]

@tig the v2_develop packages are hard to find. Can you help me find them, please. Thanks.

They're literally listed at the top of the page: https://www.nuget.org/packages/Terminal.Gui/

[tznind]

Re: nuget. Why not have TGD use -v2_develop packages that are published until alpha is released?

Yes, I am targetting 2.0.0-v2-develop.2168 👍

[BDisp]

Re: nuget. Why not have TGD use -v2_develop packages that are published until alpha is released?

Yes, I am targetting 2.0.0-v2-develop.2168 👍

@tznind it's preferable to change the TG nuget package to this to automatically using always the latest version from 2.0.0 to 2.0.1 excluded:

<PackageReference Include="Terminal.Gui" Version="[2.0.0-v2-develop.2168,2.0.1)" />

[tznind]

it's preferable to change the TG nuget package to this to automatically using always the latest version from 2.0.0 to 2.0.1 excluded

If I understand you correctly, you are suggesting to automatically target the latest nuget package?

This is not a good idea because it takes a lot of work to make TGD work with each new nuget package and changes tend not to be backwards compatible.

So targetting a specific release is currently the approach I am currently taking:

• update nuget package reference
• mark any new views that break things as unsupported
• fix any unit tests failing
• fix any renamed or redesigned properties
• merge to v2 branch

This way a user can target a specific nuget package version that they know works with TGD and each version of TGD only works with that specific version.

When we have an API that is backwards compatible (i.e. past Beta) it will make more sense to support multiple versions.

Users of v2 who want designer support currently have to

• Target the same nuget package as TGD v2 branch head

Each time TGD gets updated to target a new package they must

• Update their nuget reference to match the new compatible release
• Make manual changes to their .Designer.cs files for anything broken in the compatibility layer

But this is to be expected in pre alpha.

[BDisp]

Laud and clear. Thanks.

[tznind]

Gonna close this because seems related to parallel test execution [Parallelizable( ParallelScope.Children )] which is not supported.

[dodexahedron]

Re: nuget. Why not have TGD use -v2_develop packages that are published until alpha is released?

Yes, I am targetting 2.0.0-v2-develop.2168 👍

@tznind it's preferable to change the TG nuget package to this to automatically using always the latest version from 2.0.0 to 2.0.1 excluded:

<PackageReference Include="Terminal.Gui" Version="[2.0.0-v2-develop.2168,2.0.1)" />

Note that, with version strings, having anything non-numeric following the final number automatically opts you in to prerelease packages.

You can use a version like "2.0-literallyAnyText" instead of the range and it will pull the latest 2.0 prerelease. Why did they do it implicitly like that instead of having an attribute to opt in? One of life's mysteries....