Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
muencht opened this issue Jun 2, 2024 · 19 comments
Open
Labels
Milestone

Comments

@muencht
Copy link

muencht commented Jun 2, 2024

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
Copy link
Collaborator

BDisp commented Jun 2, 2024

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
Copy link
Author

muencht commented Jun 3, 2024

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
Copy link
Collaborator

BDisp commented Jun 3, 2024

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
Copy link
Collaborator

dodexahedron commented Jun 3, 2024

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
Copy link
Collaborator

tznind commented Jun 3, 2024

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:

  • No native process output should be written to the console (i.e. you must capture all I/O streams)
  • Any async calls that update the UI (e.g. events from process) need to be wrapped with See Application.MainLoop.Invoke

@BDisp
Copy link
Collaborator

BDisp commented Jun 3, 2024

@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
Copy link
Collaborator

BDisp commented Jun 3, 2024

@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
Copy link
Author

muencht commented Jun 3, 2024

@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
Copy link
Collaborator

BDisp commented Jun 4, 2024

@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
Copy link
Collaborator

BDisp commented Jun 4, 2024

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
Copy link
Collaborator

tig commented Jun 4, 2024

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

@dodexahedron
Copy link
Collaborator

dodexahedron commented Jun 4, 2024

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
Copy link
Collaborator

BDisp commented Jun 4, 2024

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
Copy link
Collaborator

dodexahedron commented Jun 4, 2024

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
Copy link
Collaborator

BDisp commented Jun 4, 2024

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
Copy link
Author

muencht commented Jun 5, 2024

@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
Copy link
Collaborator

BDisp commented Jun 5, 2024

$ 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.

@tig tig added the bug label Jun 9, 2024
@tig tig added this to the v1.17.1 milestone Jun 9, 2024
@tig
Copy link
Collaborator

tig commented Jul 11, 2024

Is this a real bug?

@BDisp
Copy link
Collaborator

BDisp commented Jul 11, 2024

Is this a real bug?

I don't think so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 🆕 Not Triaged
Development

No branches or pull requests

5 participants