jamulus icon indicating copy to clipboard operation
jamulus copied to clipboard

Jamulus -h does not terminate correctly in Windows cmd console

Open drummer1154 opened this issue 4 years ago • 19 comments

Describe the bug When 'jamulus -h' is issued in a cmd console window, the help text is displayed below the next command prompt. I.e. first the next prompt appears, and only then the current help text is displayed. The 2nd line in Screenshots below shows the problem: "C:\Program Files (x86)\Jamulus>" must not be appended by "Usage: jamulus [option] [optional argument]". "C:\Program Files (x86)\Jamulus>" shall be displayed BELOW the help text. Currently you need to enter Ctrl+C or return to clean up and get the next (empty) command prompt. Same problem for jamulus -v

To Reproduce

  • open a cmd console (e.g. press WINDOWS+R, enter cmd, press return)
  • enter cd "C:\Program Files (x86)\Jamulus" including the double quotes which will display the following prompt C:\Program Files (x86)\Jamulus>
  • enter jamulus -h

Expected behavior The help text shall be displayed, followed by an empty new line prompt, e.g. C:\Program Files (x86)\Jamulus>jamulus -h Usage: jamulus [option] [optional argument] : Example: jamulus -s --inifile myinifile.ini C:\Program Files (x86)\Jamulus>

Screenshots C:\Program Files (x86)\Jamulus>jamulus -h C:\Program Files (x86)\Jamulus>Usage: jamulus [option] [optional argument] Recognized options: -h, -?, --help display this help text and exit : Example: jamulus -s --inifile myinifile.ini

Operating system Windows 8.1 and above

Version of Jamulus 3.6.2

Additional context I think the output of tsConsole needs to be flushed prior to exiting the main program.

drummer1154 avatar Mar 16 '21 23:03 drummer1154

Thanks for reporting!

I think the output of tsConsole needs to be flushed prior to exiting the main program.

Cool that you already have an idea what the problem could be. Have you tried doing that already? Would be cool if you opened a PR then? :)

hoffie avatar Mar 16 '21 23:03 hoffie

Sorry, but I do not have a local Windows build environment. I have created a Jamulus server on a GCP Debian VM instance. I'm not a UX expert, having only some C but virtually no C++ background. On Linux, the help text is displayed and the prompt follows, no issue there.

drummer1154 avatar Mar 17 '21 00:03 drummer1154

Maybe Windows(tm) command.exe is at fault here. If Jamulus is at fault, maybe it is because exit(0) is used. Try exit(1) t see what happens. Otherwise, Jamulus may need a flush() and a sleep() before exitting.

ghost avatar Mar 17 '21 01:03 ghost

I believe adding flush() and sleep() will fix the problem on Windows and does not harm on Linux - performance is not crucial for help text display.

drummer1154 avatar Mar 17 '21 12:03 drummer1154

It's a "Qt on Windows" issue - i.e. partly one, partly the other - from what I remember of my investigations into this.

If you run a program from Windows, if it's meant to run in a GUI, then it immediately releases the console so you can close the console.

That's why some Windows programs put their --help text into a popup window - it's "more natural", as the command prompt still reappears, without getting overwritten by help text.

The problem is that the console has already written out the next prompt -- it's just waiting for you to enter the command. flush() won't do anything as there is nothing buffered by Jamulus (or, indeed, Windows) waiting to be written. sleep() won't do anything, either.

pljones avatar Mar 18 '21 17:03 pljones

Maybe the Windows version can prepend a newline to messages so that, at least, the message starts on a new line rather than at the end of the prompt. Furthermore, pressing ctrl-c to exit on Windows will keep you sharp.

ghost avatar Mar 18 '21 18:03 ghost

Would it help to play around with this logic? https://github.com/jamulussoftware/jamulus/blob/976ad09cf0806842800cce4b6dee175065e9c5d2/src/main.cpp#L92

I'm thinking of not only doing it for headless builds, but only doing it when the GUI launches, i.e. after validating command line arguments and knowing that a GUI will launch at all (i.e. no -n).

hoffie avatar Mar 18 '21 20:03 hoffie

From my PoV - if the application determines that it will not create a GUI because it only needs to print some text to stdout then there is no need to create a separate console process for this. Maybe I am wrong - but I think a separate console process is (only) required if you want to establish a second channel to communicate with the user besides the GUI, e.g. for debugging. Is this the case for Jamulus?

drummer1154 avatar Mar 19 '21 01:03 drummer1154

if (AttachConsole(ATTACH_PARENT_PROCESS)) { looks more like it's reacquiring the console so it can write to it, rather than detaching, which is the behaviour we're observing. Without that, I think the qInfo() output was simply vanishing: https://github.com/jamulussoftware/jamulus/blob/976ad09cf0806842800cce4b6dee175065e9c5d2/src/main.cpp#L538 (and others)

pljones avatar Mar 19 '21 22:03 pljones

A very dumb attempt at moving the AttachConsole call is in Draft PR #1401. Edit: Doesn't work as tested by @npostavs :(

hoffie avatar Mar 30 '21 21:03 hoffie

if (AttachConsole(ATTACH_PARENT_PROCESS)) { looks more like it's reacquiring the console so it can write to it, rather than detaching, which is the behaviour we're observing. Without that, I think the qInfo() output was simply vanishing:

https://github.com/jamulussoftware/jamulus/blob/976ad09cf0806842800cce4b6dee175065e9c5d2/src/main.cpp#L538 (and others)

@pljones: What exactly do you want to tell us with this input? I do not see any answer to my question https://github.com/jamulussoftware/jamulus/issues/1281#issuecomment-802429487 - is it asking too much?

drummer1154 avatar Apr 12 '21 23:04 drummer1154

I'll refer you to my answer if you're rude enough to just refer me to your question. I answered it. Read it and try UNDERSTANDING it rather than rejecting it because you can't be bothered.

pljones avatar Apr 13 '21 07:04 pljones

From my PoV - if the application determines that it will not create a GUI because it only needs to print some text to stdout then there is no need to create a separate console process for this.

I think the confusion here is that Windows applications have to decide at compile time whether they are GUI or console initially. Jamulus.exe is a GUI application, so it doesn't have console access when it starts up. If it wants to access console output then it needs to take some special action (i.e., AttachConsole).

Anyway, from what I understand, the status at present is that there is no straightforward way of fixing this bug. qInfo has no flush operation, but even switching to std::cerr and using std::flush didn't work any better for me. IMO, going with wontfix would be fine; the bug isn't really that much of a problem in practice.

npostavs avatar Apr 13 '21 12:04 npostavs

if (AttachConsole(ATTACH_PARENT_PROCESS)) { looks more like it's reacquiring the console so it can write to it, rather than detaching, which is the behaviour we're observing. Without that, I think the qInfo() output was simply vanishing: https://github.com/jamulussoftware/jamulus/blob/976ad09cf0806842800cce4b6dee175065e9c5d2/src/main.cpp#L538

(and others)

@pljones: What exactly do you want to tell us with this input? I do not see any answer to my question #1281 (comment) - is it asking too much?

I think @pljones simply mentioned that he did not think that changing AttachConsole() was the right approach and even if it was that it would possibly have side effects. Further tests have confirmed exactly what @pljones already suspected.

So the status quo is: This issue sounds like something which would be quick to solve, but it apparently isn't. So unless someone comes up with an idea how to solve this, little is going to happen (although several people have made attempts at solving this). In other words, it needs an idea which does not break anything else. If there is one, it's likely to be implemented. :)

hoffie avatar Apr 13 '21 12:04 hoffie

I think this summarizes the situation well: https://stackoverflow.com/questions/15952892/using-the-console-in-a-gui-app-in-windows-only-if-its-run-from-a-console

The command processor pays attention to your EXE and checks if it is console mode app or a GUI app. It will detect a GUI app in your case and then doesn't wait for the process to complete. It displays the prompt again and waits for input.

There's a simple workaround for that, your user should start your program with start /wait yourapp to tell the command processor to wait for the process to complete. Problem is: nobody ever uses that.

Only two good ways to solve this unsolvable problem. Either build your program as a console mode app and call FreeConsole() when you find out you want to display a GUI. Or always call AllocConsole(). These are not great alternatives. The first approach is the one used by the Java JVM on Windows. One of the oldest bugs filed against the JVM and driving Java programmers completely batty from the flashing console window.

The third alternative is the only decent one, and the one you don't want, create another EXE that will always use the console. Like Java does, javaw.exe vs java.exe.

A trick is possible, you can rename that file from "yourapp2.exe" to "yourapp.com". It will be picked first when the user types "yourapp" at the command line prompt, a desktop shortcut can still point to "yourapp.exe". Visual Studio uses this trick, devenv.com vs devenv.exe.

npostavs avatar Apr 14 '21 17:04 npostavs

it needs an idea which does not break anything else. If there is one, it's likely to be implemented.

As of today I can only confirm that in Release 3.8.0 the odd behavior still exists.

drummer1154 avatar Jun 09 '21 19:06 drummer1154

it needs an idea which does not break anything else. If there is one, it's likely to be implemented.

As of today I can only confirm that in Release 3.8.0 the odd behavior still exists.

The Jamulus -h improvement #1281 was not applied to Release 3.8.0

ghost avatar Jun 09 '21 19:06 ghost

The Jamulus -h improvement #1281 was not applied to Release 3.8.0

No doubt about that - as there is no improvement available for the time being ;-) I just did my usual test after installing 3.8.0 today on Win/Mac/Debian. Cheers Helmuth

drummer1154 avatar Jun 09 '21 20:06 drummer1154

Hi, Sorry for the maintenance noise here. I’m just into triaging issues.

I think this issue is still present, even on the latest Windows release? @drummer1154 @npostavs ? If this is NOT the case, please close this issue.

Note: I will unsubscribe from this issue and won’t receive responses from any new comments. If you have any questions concerning maintenance, feel free to ping me.

ann0see avatar Apr 22 '22 19:04 ann0see

I believe this issue should be closed or at least the title should be changed. This is just how to combination of a GUI application and Console option work with the current implementation and believe it needs a lot of work to change that.

Also the command is terminated correctly, the only thing not happening is the (re)print of the prompt. CTRL+C is not needed at all (which would terminate a running program). You can just start typing the next command you want to run, which proofs there is nothing to terminate.

henkdegroot avatar Aug 29 '22 19:08 henkdegroot

I agree. Multiple people have tried tackling this really minor cosmetic annoyance. It's not worth tracking it any longer. If someone comes up with a PR with no unwanted side effects, I'd be happy to take it. Closing, also based on @henkdegroot's judgement and @pljones' +1.

hoffie avatar Aug 29 '22 20:08 hoffie