Page MenuHome

Remove Close From Console Window
ClosedPublic

Authored by Harley Acheson (harley) on Apr 1 2019, 2:11 AM.

Details

Summary

This patch removes the "close" button on the top-right of the console window (Windows)

This way if you toggle the console, closing the resulting window does not close Blender.

Diff Detail

Repository
rB Blender

Event Timeline

Harley Acheson (harley) retitled this revision from Don' to Remove Close From Console Window.
Harley Acheson (harley) edited the summary of this revision. (Show Details)

Hate to sounds like an xkcd caricature , but that's generally the way i kill blender whenever it's hung or otherwise real unhappy with the state of things.

Could we do something like:

https://docs.microsoft.com/en-us/windows/console/registering-a-control-handler-function

To intercept the close and/or CTRL-C and do a simple alert dialog (basically the same as the current Windows "Do you really want to quit?" on exit)

@LadyDodo: Love xkcd "workflow" one! But there is always Task Manager. ;-) But note that this does NOT disable the close if the Blender session was started from a command prompt.

@Gavin Scott (Zoot): Maybe. But I was too lazy to investigate how that through. It might be just a bit more complicated since we are always in a console app, just normally hidden. So a handler on close might be triggered always, so then we'd have to show the dialog only if console was hidden. Again, probably not a biggie. But just disabling the "close" would eliminate most errors.

Or we might decided that doing this, just disabling the close, might not be enough. But this seemed quick and easy

But note that this does NOT disable the close if the Blender session was started from a command prompt.

Ah right. I think this is definitely worthwhile then since when someone finds that "Toggle system console" menu item for the first time it almost always results in them terminating Blender a few moments later :)

Seems like a good solution.

intern/ghost/intern/GHOST_SystemWin32.cpp
1733

If we don't want to delete button when started from the command prompt, shouldn't we test if(!isStartedFromCommandPrompt()) { here?

I guess there is some indirect reason it doesn't happen, but seems better to be explicit?

Brecht, you are exactly right: it should be explicit.

The way the patch is right now, you could start Blender from an existing command prompt (so there is an enabled close button), but then "Toggle System Console" twice within blender and then close would be disabled. Not ideal I think. Will make an improved patch.

The big reason though I did not use that isStartedFromCommandPrompt() is because it seems a bit broken in that it returns a result opposite to its name. You can see that in that existing comment "startup: hide if not started from command prompt" even though it clearly hides if that is true, not false. It seems to work properly (if you ignore the name), but I worried that using that function more will make that section harder to understand.

But will submit a new patch that works properly, without making any changes to that isStartedFromCommandPrompt() name.

This change makes it so the console "close" button is only ever disabled if Blender was not started from a command prompt.

The prior patch worked as expected if you just launch Blender normally and then select "Toggle System Console". However, if you started initially from an existing command prompt you don't want that toggle to disable close.

Note however that this change looks confusing...

The function isStartedFromCommandPrompt() returns the opposite of what you'd expect. You can see that in the code that existed prior to this patch. On line 1717 you see the comment that it will "hide if not started from command prompt" and that is what it does, but you see that it is hiding if the result of isStartedFromCommandPrompt() is true, not false.

So this patch just uses that function as named and written so it looks a bit confusing. But didn't want to change that function as it seemed outside the scope of this patch. And I don't understand the internals of that function.

Harley Acheson (harley) marked an inline comment as done.Apr 2 2019, 8:27 PM

Having a quick look at isStartedFromCommandPrompt()...

It definitely looks like the intention of the function matches the name, in that appears to be made to see if the Blender process was launched from within an existing command prompt.

The default return of false seems reasonable.

If there is a handle returned from GetConsoleWindow then our current process has a command window, either we are a command-line app (yes) or we spawned one. So that section (1692-1713) looks fine.

Then there is a section that tries to see if we have a parent process with a name of "blender.exe". This is always fails and I think always should. Can't think of when start_from_launcher would ever be true but maybe it is from something older or a feature I'm not aware of. Like if Blender spawned the command prompt as a child process.

So the meat of the function comes down to line 1711. With start_from_launcher always false, this means the test is: return true if the current process Id is equal to the processId of the console window. This the part that is backwards. If the process Id of the console window is the same as the current process Id that means we are in the same process and therefore we did NOT start from an existing command prompt.

Therefore the correction is that the equality test on 1711 should be changed to inequality. Ie:

if (pid != (start_from_launcher ? ppid : GetCurrentProcessId())) return true;

And then reverse the usages of this test to match: just used once before this patch, twice more added with this patch.

intern/ghost/intern/GHOST_SystemWin32.cpp
1733

Yes, the new patch does check for this. But note my comments about that function as it returns the opposite result that the one expected

Harley Acheson (harley) marked an inline comment as done.Apr 2 2019, 8:38 PM
Harley Acheson (harley) updated this revision to Diff 14533.

So in case you want this done, this is the same patch but also CORRECTS the issue with isStartedFromCommandPrompt() so that function can be used as the name implies.

LazyDodo (LazyDodo) added a comment.EditedApr 2 2019, 8:39 PM

Just fix the function so it does what you expect it to do, and fix the one call site that was using it wrong.

edit: already done, that was fast :)

Sorry to be so... wordy. Never know how much detail to get into with these things. LOL

This revision is now accepted and ready to land.Apr 3 2019, 6:04 PM
This revision was automatically updated to reflect the committed changes.