gui-cs / Terminal.Gui

Cross Platform Terminal UI toolkit for .NET
MIT License
9.35k stars 673 forks source link

Terminal.Gui 1.17.0 Throws System Exception in Application.Run(dialog) #3526

Open muencht opened 1 month ago

muencht commented 1 month ago

Describe the bug I am creating, for nostalgia purposes, an Emacs-like text-mode editor using Terminal.Gui and the UICatalog sample Editor.cs . I am unbinding a lot of the default TGui keys and making it a lot more "real" Emacs but without multiple buffers, split-screen, etc.

One thing I implemented was Emacs shell-command which shells the DOS command specified by the user and displays the output in a popup, here a TGui Dialog. It was all working fine and then all of a sudden the code threw an Exception. I spent hours trying to track it down, finally had to downgrade TGui from v1.17.0 (laatest). to v1.16.0 . This fixed the problem.

To Reproduce Steps to reproduce the behavior:

  1. Create a C# Console (Core) Solution with Program.cs using .NET Core 6.0
  2. Add Class file Popup.cs (source below) to the solution using Terminal.Gui;
  3. In Program.cs Main() add the following lines Popup popup = new Popup(text); <= BREAKPOINT popup.Show(); //shows the dialog
  4. Build and Run the Solution with Debug
  5. At the above Breakpoint Step Into Popup
  6. Run down to line Application.Run(dialog)
  7. Step Over this line will get Exception

Expected behavior I expect the dialog to open with the shell-command output in a ListView.

Actual behavior An Exception is thrown

File: Popup.cs Line: Application.Run(dialog); System.ArgumentException: 'start' This exception was originally thrown at this call stack: [External Code] Terminal.Popup.Show() in Popup.cs Terminal.Program.Emacs.shell_command() in Program.cs at NStack.ustring.Byte ... (Source:NStack

Screenshots If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information): VS 2019 w/ .NETCore 6.0 Terminal.Gui v1.17.0

Additional context

//////////////////////////////////////////////////////////////////////////////////////////////////// // // Class: popup // // A popup derived from Dialog; the purpose here is to display output from shell-command // // Popup popup = new Popup(string text) // popup.Show() shows the dialog // ////////////////////////////////////////////////////////////////////////////////////////////////////

using System; using System.Collections.Generic; using System.Linq; using Terminal.Gui;

namespace Terminal { class Popup {

    // properties
    public string _text { get; set; }

    // Constructors
    public Popup() { }

    public Popup(string text) {
        _text = text;
    }

    public void Show() {
        Dialog dialog = new Dialog {
            Title = "Output from shell-command",
            Width = 65,
            Height = 45,
            Border = new Border() {
                BorderStyle = BorderStyle.Rounded,
                DrawMarginFrame = true,
                Effect3D = false,
            },
        };

        // convert text to List<>
        List<string> list = _text.Split(new[] { "\r\n" },
            StringSplitOptions.None).ToList();

        ListView listView = new ListView(list) {
            AllowsMultipleSelection = true,
            LayoutStyle = LayoutStyle.Computed,
            TextAlignment = TextAlignment.Centered,
            X = Pos.At(5), 
            Y = Pos.At(3), 
            Width = Dim.Fill(2),
            Height = Dim.Percent(80) 
        };
        listView.AutoSize = true;
        dialog.Add(listView);

        // btn to copy to clipboard
        Button btnCopy = new Button(1, 30, "Copy");
        btnCopy.X = Pos.At(73); 
        btnCopy.Y = Pos.At(35); 

        string allText = "";
        btnCopy.Clicked += () => {
            allText = "";
            for (int i = 0; i < list.Count; i++) {
                listView.SelectedItem = i;
            }
            // copy to clipbd
            System.Windows.Forms.Clipboard.SetText(allText);
            MessageBox.Query(30, 6, "", "\r\nText copied to clipboard.", "Ok");
        };

        listView.SelectedItemChanged += (args) => {
            allText += list[listView.SelectedItem] + "\r\n";
        };
        dialog.Add(btnCopy);

        // btn to clisk when done
        Button btnClose = new Button(1, 20, "Close");
        btnClose.X = Pos.At(35); //32 30 //Bottom(_editor);
        btnClose.Y = Pos.At(38); // 41 39 40);

        btnClose.Clicked += () => {
            Application.RequestStop();
        };
        dialog.Add(btnClose);

        dialog.Height = Dim.Sized(42); 
        dialog.Width = Dim.Sized(85); 

        dialog.AutoSize = true;
        dialog.Visible = true;

        try {
            Application.Run(dialog);
        } catch (Exception ex) {
            // is ignored
            MessageBox.Query(30, 6, "Error", ex.Message, "Ok");
        }
    }
}

}

BDisp commented 1 month ago

This Main code works:

            Application.Init ();

            var popup = new Popup ("Popup test!");
            popup.Show ();

            Application.Shutdown ();

In the Popup class you have to change the line System.Windows.Forms.Clipboard.SetText (allText); with Clipboard.Contents = allText;.

muencht commented 1 month ago

I am using System.Windows.Forms.Clipboard.SetText() because I have using System.Windows.Forms; in Program.cs

Again my code works as is with 1.16.0 not 1.17.0

Why should it matter? I can try your Clipboard.Contents = allText; in 1.17.0 but if what I have fails I think it should fail in both.

BDisp commented 1 month ago

Where you are using Application.Init ();? This is important to initialize the driver. Can you try using net 7.0 or net 8.0?

dodexahedron commented 1 month ago

v1 calls InternalInit, which sets up the driver, in Run, ultimately, specifically when it reaches Run<T>, if _initialized == false.

If he's running it within his application already, the driver should already be set up. He's got a working application, so that's already the case. He's just hitting the problem on trying to show a new dialog.

I've encountered that same sort of issue in v1.

Generally, showing a Dialog in an already-running application seems to have that problem, in certain scenarios that I've not traced (I had actually asked a similar question quite a while back, before I started helping out with V2, in fact), and it's avoidable by using a plain Window instead of a Dialog and just using Show instead of Application.Run on it.

Window.Modal can be set if a modal "dialog" type behavior is wanted.

Application.Driver is static, so calling Init again will not do anything new with the driver.

tznind commented 1 month ago

One thing I implemented was Emacs shell-command which shells the DOS command specified by the user and displays the output in a popup

How are you executing the process? There are some important things to consider:

BDisp commented 1 month ago

@dodexahedron if I remember correctly you used a class derived from Toplevel where you used the Show method without having set the IsMdiContainer property to true, because this method is only used in these conditions. As you used Application.Run<T> then the driver was automatically loaded. In this case, the Popup class is not derived from View and contains the Show method to create a Dialog that is executed by calling Application.Run(dialog);, hence my question. Unless Application.Init has already been called explicitly or implicitly, in another routine that @muencht did not show. But it appears that the exception references 'start' and @tznind may be right in thinking that this is being executed through a process.

BDisp commented 1 month ago

@muencht the bellow code works using System.Windows.Forms and Terminal.Gui v1.17.0. Now your Popup class is derived from Dialog. Maybe the error is due to the shell-command and thus can you provide the code how you are calling from it, please. Thanks

.csproj:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0-windows</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <UseWindowsForms>true</UseWindowsForms>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Terminal.Gui" Version="1.17.0" />
  </ItemGroup>

</Project>

Program.cs:

using Terminal;

internal static class Program
{
    [STAThread]
    static void Main()
    {
        new Popup($"Popup{Environment.NewLine}Dialog{Environment.NewLine}Test!");
    }
}

Popup.cs:

////////////////////////////////////////////////////////////////////////////////////////////////////
//
// Class: popup
//
// A popup derived from Dialog; the purpose here is to display output from shell-command
//
// new Popup(string text)
//
////////////////////////////////////////////////////////////////////////////////////////////////////

using Terminal.Gui;
using Application = Terminal.Gui.Application;
using BorderStyle = Terminal.Gui.BorderStyle;
using Button = Terminal.Gui.Button;
using ListView = Terminal.Gui.ListView;
using MessageBox = Terminal.Gui.MessageBox;

namespace Terminal
{
    class Popup : Dialog
    {
        // Constructors
        public Popup() : this (String.Empty) { }

        public Popup(string text)
        {
            Application.Init();

            Title = "Output from shell-command";
            Width = Dim.Percent(70f);
            Height = Dim.Fill();
            Border = new Border()
            {
                BorderStyle = BorderStyle.Rounded,
                DrawMarginFrame = true,
                Effect3D = false,
            };
            ColorScheme = Colors.Dialog;

            // convert text to List<>
            List<string> list = text.Split(new[] { Environment.NewLine },
                StringSplitOptions.None).ToList();

            ListView listView = new ListView(list)
            {
                AllowsMultipleSelection = true,
                LayoutStyle = LayoutStyle.Computed,
                TextAlignment = TextAlignment.Centered,
                X = Pos.At(5),
                Y = Pos.At(3),
                Width = Dim.Fill(2),
                Height = Dim.Percent(80)
            };
            Add(listView);

            // btn to copy to clipboard
            Button btnCopy = new Button("Copy")
            {
                X = Pos.AnchorEnd(8),
                Y = Pos.AnchorEnd(2)
            };

            string allText = "";
            btnCopy.Clicked += () => {
                allText = "";
                for (int i = 0; i < list.Count; i++)
                {
                    listView.SelectedItem = i;
                }
                // copy to clipbd
                System.Windows.Forms.Clipboard.SetText(allText);
                MessageBox.Query("Text copied to clipboard.", $"{Environment.NewLine}{allText}", "Ok");
            };

            listView.SelectedItemChanged += (args) => {
                allText += list[listView.SelectedItem] + Environment.NewLine;
            };
            Add(btnCopy);

            // btn to click when done
            Button btnClose = new Button("Close")
            {
                X = Pos.Center(),
                Y = Pos.AnchorEnd(1)
            };

            btnClose.Clicked += () => {
                Application.RequestStop();
            };
            Add(btnClose);

            try
            {
                Application.Run(this);
            }
            catch (Exception ex)
            {
                // is ignored
                MessageBox.Query(30, 6, "Error", ex.Message, "Ok");
            }

            Application.Shutdown();
        }
    }
}

devenv_FIMg0y0Ws5

muencht commented 1 month ago

@BDisp thanks for your time spent on this. I made the following changes to class Popup

  1. class Popup : Dialog (derived from) <=

    better_ but I think what I had should be ok I have done this many times in core C#

  2. eliminated .Show() (I like it)(kept it)

    this is the way C# does it Form.Show()

  3. has Application.Init() in class <=

    is this required in every class that uses TGui? .. not just the main class (here Program)

  4. Application.Run(this) <=

  5. added Application.Shutdown() at end <=

Test #1 (1.16.0) Runs

Test #2 (1.17.0) System.ArgumentException: 'start'

Test #3 (1.17.0) Removed .Show() and put all code in constructor as you have it System.ArgumentException: 'start'

You said "Maybe the error is due to the shell-command and thus can you provide the code how you are calling from it, please. "

I can't believe this is the problem; I am using the standard System.Diagnostics.Process spawn ... If it is the problem, how can it work in 1.16.0 -?-

The only other difference I can see is your class Program

[STAThread] static void Main() { new Popup($"Popup{Environment.NewLine}Dialog{Environment.NewLine}Test!"); }

where as I said I invoke thus

Popup popup = new Popup(output);

I'm tired of fighting this and will use 1.16.0 until a new version comes out (is one planned?). However I will try a suggestion from @dodexahedron who says he has encountered this problem and

"it's avoidable by using a plain Window instead of a Dialog and just using Show instead of Application.Run on it."

BDisp commented 1 month ago

@BDisp thanks for your time spent on this. I made the following changes to class Popup

You welcome.

  1. class Popup : Dialog (derived from) <= better_ but I think what I had should be ok I have done this many times in core C#

But in your case you can't say that is derived from Dialog because it's a class that create a Dialog in the Show method. So it is different.

  1. eliminated .Show() (I like it)(kept it) this is the way C# does it Form.Show()

I understand. In this case you can only move the run code to the Show method, like this:

        public void Show()
        {
            try
            {
                Application.Run(this);
            }
            catch (Exception ex)
            {
                // is ignored
                MessageBox.Query(30, 6, "Error", ex.Message, "Ok");
            }
            finally
            {
                Application.Shutdown();
            }
        }
  1. has Application.Init() in class <= is this required in every class that uses TGui? .. not just the main class (here Program)

Application.Init() is only required once at your start application. I put in the Popup class because I supposed you have only that class which run TGui. It's only required when you use the parameter-less method Application.Run() or the Application.Run(MyTopView). If you use the generic one Application.Run<MyTopView>() you don't need to use it because the InternalInit will be call to initialize the driver.

  1. Application.Run(this) <=

Right, now it's a derived class from Dialog and so the this is used.

  1. added Application.Shutdown() at end <=

Application.Shutdown() is also need to call once when the application exit which perform cleanup and restore the terminal. I put it in the Popup class because I supposed you only that use it, otherwise must be in the class which start calling the TGui.

Test #1 (1.16.0) Runs

Test #2 (1.17.0) System.ArgumentException: 'start'

In my case with the same code as I already submitted using the TG v1.17.0, is running without error. You are running on Windows in a terminal? Do you are running on another application and call the Popup app with a Process.Start? You need to provide more information because running as your instructions I don't have any exception. What is your OS?

Test #3 (1.17.0) Removed .Show() and put all code in constructor as you have it System.ArgumentException: 'start'

That is expected, because the execution is the same.

You said "Maybe the error is due to the shell-command and thus can you provide the code how you are calling from it, please. "

I can't believe this is the problem; I am using the standard System.Diagnostics.Process spawn ... If it is the problem, how can it work in 1.16.0 -?-

That's a start. May be is the cause but please explain how you are using the System.Diagnostics.Process spawn to I could perform some test. Thus maybe some of TGui v1.17.0 code that was changed from the previous version is causing this, but you need to tell me how you are proceeding to achieve that error. That way maybe I can help you.

The only other difference I can see is your class Program

[STAThread] static void Main() { new Popup($"Popup{Environment.NewLine}Dialog{Environment.NewLine}Test!"); }

where as I said I invoke thus

Popup popup = new Popup(output);

I had to use the [STAThread] to avoid the exception on the call System.Windows.Forms.Clipboard.SetText(allText);. Did you use it too?

I'm tired of fighting this and will use 1.16.0 until a new version comes out (is one planned?). However I will try a suggestion from @dodexahedron who says he has encountered this problem and

"it's avoidable by using a plain Window instead of a Dialog and just using Show instead of Application.Run on it."

I think it would not fix your issue, because it was another problem with a windows refresh not being centering a Window because he was calling the Show method that belongs to the Toplevel of the TGui, which only work if the Toplevel is a MdiContainer and also had some TGui bugs that was then fixed. But if you want to make a quick test you can change your derived class to Popup : Window or Popup : Toplevel.

BDisp commented 1 month ago

I created another console project that call the Popup from a process and it run without throwing any exception, by debugging or launching from a terminal.

.csproj:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

</Project>

Program.cs:

using System.Diagnostics;

try
{
    using Process process = new Process();
    process.StartInfo.UseShellExecute = false;
    process.StartInfo.FileName = @"..\..\..\..\PopupDialog\bin\Debug\net6.0-windows\PopupDialog.exe";
    process.Start();
    process.WaitForExit();
}
catch (Exception e)
{
    Console.WriteLine(e.Message);
}

image

tig commented 1 month ago

I suggest looking at the changes from v1.16 to v1.17 and backing each potential cause out to see if you can figure out what caused the change in behavior.

https://github.com/gui-cs/Terminal.Gui/commit/d774f622a16c543987622fdd1119c1d2f36b1024

dodexahedron commented 1 month ago

I suggest looking at the changes from v1.16 to v1.17 and backing each potential cause out to see if you can figure out what caused the change in behavior.

d774f62

For anyone unfamiliar with the feature, you can use the blame feature in git or github to do exactly this.

It's kind of a silly named command, but fits right in with how git itself was named in the first place. 😆

(Git is pronounced "jit," as in short for "idiot." Thank Linus Torvalds for that one.)

BDisp commented 1 month ago

But someone can explain to me why my code that is using the same source and the same TG version isn't throwing any exception? The only main changes that could affect is the adding of the net 8.0 and the netstandard 2.0. But my code isn't failing at all.

dodexahedron commented 1 month ago

That's a good question, and really just requires debugging in-situ plus walking back through the commits to find out what's going on.

Chances are it's just some subtle issue in the application's code which was uncovered by or made more likely to happen by fixes in TG, rather than an actual new bug in TG, but only tracing it can answer it definitively.

There are some idiosyncrasies with v1 that don't have supporting documentation and which can be very unintuitive for various reasons, especially as the consuming application grows in size and complexity. 🤷‍♂️

BDisp commented 1 month ago

I can't debug it to find the cause of an anomaly if I don't have the anomaly that @muencht reported. I believe he is facing this dilemma but I was unable to reproduce it to be able to detect it.

muencht commented 1 month ago

@BDisp thanks again for all your time; I am dopping this for now, using v1.16

In reply to your last lengthy reply

You say: Application.Init() is only required once at your start application. I put in the Popup class because I supposed you have only that class which run TGui.

Application.Init() is in Program.cs; this is a retro text editor -- many classes use Terminal.gui !!!

You say: You are running on Windows in a terminal? Do you are running on another application and call the Popup app with a Process.Start? You need to provide more information because running as your instructions I don't have any exception. What is your OS?

OS = Windows 11; I execute exe headless from Windows Terminal

$ wt -f --pos "200,100" Terminal.exe ......\doc\bug-report.txt

** You are mistaken thinking I "call the Popup app with Process.Start." The shell/spawn has nothing to do with Popup; Popup only displays the text output from the spawn.

** I wrote a test program for the Process.Start; it runs w/o error on 1.17.0

You say: I had to use the [STAThread] to avoid the exception on the call to

System.Windows.Forms.Clipboard.SetText(allText);. // Did you use it too? 

YES--

[STAThreadAttribute]
public static void Main(string[] args) {

    Terminal.Gui.Application.Init();
    ...
}

I say: @dodexahedron says he has encountered this problem and "it's avoidable by using a plain Window instead of a Dialog and just using Show instead of Application.Run on it."

You say "I think it would not fix your issue, because it was another problem with a windows refresh not being centering a Window because he was calling the Show method that belongs to the Toplevel of the TGui, which only work if the Toplevel is a MdiContainer and also had some TGui bugs that was then fixed. "

You are correct I'm sure; I did not waste time trying it.

## ## One last thing to try ... ##

got rid of all refs to WindowsForms FW= net6.0 not net6.0-windows set Active Config=Release clean/rebuild v1.16 ok v1.17 fails

BDisp commented 1 month ago

$ wt -f --pos "200,100" Terminal.exe ......\doc\bug-report.txt

I was able to run through the command using TG v1.17.0.

wt -f "path-to-the-exe-file"

It could be some code that is not compatible with v1.17.0, perhaps due to a fixed bug or breaking change which should not have happened. If you could share the code on github, maybe I could help.