-
Notifications
You must be signed in to change notification settings - Fork 30
feat: add support for webassembly #49
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
base: main
Are you sure you want to change the base?
Conversation
| func disableEcho(fd uintptr, state *State) error { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems rather dangerous, or at least it means that it will compile, but silently discard setting this option and (e.g.) print passwords?
Wondering if it should either error, or if we need to make it a compile option when trying to use these parts with webassembly (i.e., don't have these functions implemented).
cc @AkihiroSuda @vvoland any thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah not sure about that either.
Does k8s end up using them somehow?
If not, perhaps it would be better to omit these functions on wasm (so their usage wouldn't compile)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've try to delete https://github.com/kubernetes/kubernetes/blob/e457f5687ac58e60652b7c2d678d0aefc9bb83d4/vendor/github.com/moby/term/term.go#L58-L57 and it compiles without issue. So maybe it is not really used.
f712fd7 to
f840c9c
Compare
Signed-off-by: Rémy Léone <[email protected]>
Signed-off-by: Rémy Léone <[email protected]>
Signed-off-by: Rémy Léone <[email protected]>
Signed-off-by: Rémy Léone <[email protected]>
This PR adds support for webassembly in term. I need this patch for compiling kubectl to webassembly kubernetes/kubernetes#133638