IronLanguages / dlr

Dynamic Language Runtime
Apache License 2.0
369 stars 84 forks source link

BlazorWasm support #256

Closed TwoUnderscorez closed 2 years ago

TwoUnderscorez commented 2 years ago

When trying to use IronPython3 from a Blazor web-assembly application, Microsoft.Scripting.Runtime.SharedIO throws an exception when trying to setup stdin, which is understandable, because there is none.

All I did is catch the exception and set the stream to Stream.Null.

dnfadmin commented 2 years ago

CLA assistant check
All CLA requirements met.

TwoUnderscorez commented 2 years ago

@slozier Is that better?

TwoUnderscorez commented 2 years ago

@slozier Sorry, I don't quite understand how this is a breaking change. If you would point me in the right direction I might be able to make it a non-breaking change.

slozier commented 2 years ago

@TwoUnderscorez I meant having ConsoleInputStream.Instance go from a field to a property is technically a binary breaking change (for example if assembly A uses a field from assembly B and you replace B with B' which contains a property instead, then A is broken). I don't think I'm really worried about breaking anyone with this particular scenario since the only consumer is in the same assembly - but it is a public type so it's not impossible...

An alternative approach would be to leave ConsoleInputStream.Instance as a field and change the constructor to not throw:

private ConsoleInputStream() {
    try {
        _input = Console.OpenStandardInput();
    } catch (PlatformNotSupportedException) {
        _input = Stream.Null;
    }
}
TwoUnderscorez commented 2 years ago

@slozier I could do that, but I would have to leave the catch block in ShareIO.InitializeInput because

_inputEncoding = Console.InputEncoding;
_inputReader = Console.In;

also throw PlatformNotSupportedException (maybe only one of them), which I personally think is kind of ugly. Would you like me to do what you've suggested or merge as is currently?

slozier commented 2 years ago

@TwoUnderscorez I think the PNSE try/catch you have in InitializeInput is fine, but I'd change the ConsoleInputStream to what I suggested instead.

slozier commented 2 years ago

Thanks for the PR!