Add hide_windows_console cfg option to hide the Windows command prompt #410
Conversation
|
You can try to write this in For |
|
Thanks! This is certainly an improvement that we should make. However, one of the rules of Robrix's implementation is that we must not contain any platform-specific code, as one of the goals of Robrix is to demonstrate that an app dev can write a fully-complex app without any platform-specific code considerations. Overall, I think this change should happen within Makepad itself. This would allow others to use it too. Also, the visibility of the console window should not be tied to whether we're building in debug mode or release mode, as we very frequently test Robrix using a release build (for performance reasons), and we still want to see the console log then. This should probably be a |
There was a problem hiding this comment.
This attribute shouldn't be enabled based on whether debug_assertions are enabled, because we typically do want the console log to be visible when running Robrix in release mode on Windows.
Instead, we need to check for a custom --cfg options (such as hide_windows_console), and enable this attribute only if:
- Robrix is built using that
--cfg hide_windows_consoleconfiguration, and - if we're building for Windows (
target_os = "windows").
That way, we can enable the --cfg hide_windows_console option when we're building a distributed package of Robrix for Windows (an installable app bundle), and not just when we're building Robrix in --release mode for regular testing on Windows.
|
@yangcancai I added more context above, kindly take a look. |
|
Change it to something like this?
|
|
@kevinaboos |
|
The |
Maybe to use either the --cfg flag or a --feature to enable or disable whether the window should be hidden like below // --cfg hide_windows_console
#![cfg_attr(all(hide_windows_console, target_os = "windows"), windows_subsystem = "windows")]
// --feature hide_windows_console
#![cfg_attr(all(feature="hide_windows_console", target_os = "windows"), windows_subsystem = "windows")]However, the |
992a754 to
f8f61e0
Compare
|
yes, that's the correct way to do IMO, cargo features are not appropriate for this kind of thing because we don't need cargo-level knowledge of this feature. We could make it be a cargo feature, but it's not necessary because it doesn't affect any build-time behavior or dependency selection. I still maintain that the correct place to do this is in Makepad itself, as a special addition to the |
hide_windows_console cfg option to hide the Windows command prompt
kevinaboos
left a comment
There was a problem hiding this comment.
I made some changes and can now accept this PR. Thanks! In the future, I'll move it into Makepad itself, as discussed earlier.
No description provided.