Closed facekapow closed 9 years ago
Wait, fixing little bug... OK! The module should work now.
@ArielAbreu I think shell module shouldn't depend on runtime directly and it should be passed as an argument like require('runtime-shell')(runtime)
.
Note: runtime.shell.setCommand
is probably going to change.
I think the biggest issue with current interface is that it's not easy to redirect input/output from tty to somewhere else.
I'm also working on launching she'll commands from an application.
And about depending, that is better and it's what I use for multiple files.
The problem with dependency on runtime is that it forces everyone to use the version of runtime that is bundled with shell and not the one user would like to require.
btw runtime registers itself as a global runtime
object, that's another way to use it (no need to declare package dependency)
No, I mean (if you see the source, you'll see it too) that passing runtime in is better that's what I do to require the shell commands. Plus, I just fixed that, just need to publish it on npm.
And I didn't know it was a global, good to know.
I'd say let's figure out better interface for commands that supports input/output redirection and doesn't always push output into tty.
I'm not completely sure if global runtime object is good idea though, we might want to remove that.
Yes, a better interface is needed. Also, a way of getting Input easily from inside an app. Right now, it can be done, but you have set a listener for key down and manage keys and newlines, etc.
The global is sort of iffy, because it can be useful, but if the user wants to have runtimejs under a different name it isn't, ideal.
Opened an issue in runtime-shell https://github.com/ArielAbreu/runtime-shell/issues/1
I'm not completely sure if global runtime object is good idea though, we might want to remove that.
Me too, also but being a dependency it would allow to define with what version (of the API) of runtime it's compatible
@piranna True, but runtime-shell should be able to work any (newer) version of runtimejs. Seems logical to have whatever version the user wants.
@piranna we might want to use a peerDependency, function argument looks good to me though
I just pushed an update to npm that accepts runtime as an argument.
I published a new update to it that accepts it as a peerDependency, and makes passing runtime to the module optional.
@ArielAbreu in my opinion, if there is a peer dependency, optional argument would make it very confusing. I'd say let's just use the peerDependency and remove the argument.
Ok, i'll push the update
@piranna @kesla @ArielAbreu I'm curious what do you guys think the best way to manage plugin dependencies?
I didn't know about peerDependencies until today, I think either peerDependencies or regular dependencies, that way it can request a specific version.
@piranna True, but runtime-shell should be able to work any (newer) version of runtimejs. Seems logical to have whatever version the user wants.
Yes, but that's why semver was invented... ;-) Shell could have runtime ^0.1.0 as dependency and it will work with newer versions...
@piranna I added a ping command to shell (needs tcp), and TCP was introduced in what version? 0.1.3-0.1.4, right? Yes, it could have it as a dependency, but versions ^0.1.4 would work with it. BTW, there is a discussion issue (ArielAbreu/runtime-shell#1) in runtime-shell.
EDIT: ArielAbreu/runtime-shell#3 is better for discussion about runtime-shell.
EDIT 2: There's also a repo channel on Gitter.
while versions are <1.0 each update could be breaking, I guess peer dependency "0.x" would be ok
Closing pull request for now, it conflicts with a more important PR (#60).
@ArielAbreu how could this PR conflict with another PR?
I'm reopening it because it turns out it wasn't conflicting.
Whoops, deleted this branch by accident. That's OK! I just deleted the files. I'll do it again.
I'd like to remove the shell functions (ironic, since I added them) in favor of a module containing them (plus
ping
). This was discussed after the merge for #58 and I believe it's better to add them as a module (as suggested). I've gone ahead and created a module on NPM called "runtime-shell" containing the commands.