Skip to content

Optionally disable console by feature #5691

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

marcomq
Copy link

@marcomq marcomq commented Apr 11, 2025

Going to fix
#5690

Optionally disable stdin, stdout and stderr for example for windows GUI apps, where there is no console and where the previous code panicked.

Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Did this patch solved your problem?

One change request. Rust features are expected to be additive. In other words, feature stdio is better than disable-stdio.

@marcomq
Copy link
Author

marcomq commented Apr 12, 2025

Rust features are expected to be additive. In other words, feature stdio is better than disable-stdio.

Oh, thx. You are completely right! The docs actually mention this special case
A consequence of this is that features should be additive. That is, enabling a feature should not disable functionality, and it should usually be safe to enable any combination of features. A feature should not introduce a SemVer-incompatible change. For example, if you want to optionally support [no_std](https://doc.rust-lang.org/reference/names/preludes.html#the-no_std-attribute) environments, do not use a no_std feature. Instead, use a std feature that enables std.

Always good to learn something new :)
I will modify the PR.

@marcomq
Copy link
Author

marcomq commented Apr 12, 2025

Tbh - I think
#5420
actually looks better.
It looks like it would allow calling "print" and just prevents writing to stdout. My version would raise an error when calling "print".

@marcomq
Copy link
Author

marcomq commented Apr 12, 2025

I guess I will try to resolve the conflict in #5420 and check if it solves my issue.
Also, I'm probably going to name the feature quiet-stdio and it should disable the stdio if active.
I would prefer this for following reasons:

  • Many projects have just included RustPython-vm and stdio would just be a standard feature of RustPython but not of the VM. In those projects, they would loose stdio.
  • Some projects have already a "disable-default" and define all features that are needed manually. Those would also loose stdio.
  • The "disable-default" doesn't guarantee that a feature is actually disabled. This is usually no problem, but it might cause issues here, as the stdio might be still active due to a foreign dependency.

@marcomq
Copy link
Author

marcomq commented Apr 12, 2025

So - I tried to implement #5420 but it somehow didn't solve my issue. I still got these errors locally during initialization.
I also had a short look on the original issue - the non-UTF8 error message. But I wasn't able to locate where the error message actually originated and what the proper handling would be.

So - this is would currently be my approach of this issue.
Let me know if this is ok or if I still should change it to stdio instead of quiet-stdio

@marcomq marcomq requested a review from youknowone April 12, 2025 21:35
@arihant2math
Copy link
Collaborator

Works for me.

@coolreader18
Copy link
Member

Sorry for the delay and for the mess that this is in general - I just merged #5420; does that approach work for your problem?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants