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

Deprecate scala.sys.process.ProcessBuilder symbolic methods #11133

Open
joshlemer opened this Issue Sep 4, 2018 · 10 comments

Comments

Projects
None yet
5 participants
@joshlemer
Copy link
Member

joshlemer commented Sep 4, 2018

This was originally reported by @smarter

@joshlemer joshlemer added the library label Sep 4, 2018

@joshlemer joshlemer added this to the 2.13.0-RC1 milestone Sep 4, 2018

@SethTisue

This comment has been minimized.

Copy link
Member

SethTisue commented Sep 4, 2018

perhaps looking at Ammonite's design for their comparable API would be helpful

@SethTisue

This comment has been minimized.

Copy link
Member

SethTisue commented Oct 10, 2018

I'm unsure about the value and timing of these changes.

In general, scala.sys.process hasn't been touched much in recent years. The plan is to split it out into a separate module, but that didn't happen in time for 2.13.

I'm not sure it's desirable to change the API around so drastically before it becomes a separate, community-maintained module. I think it might be more desirable to leave the API alone in the meantime, for stability, especially this late in the 2.13 cycle.

Normally making a module proceeds first by modularizing it and doing a release without changing much, then once a backwards-compatible 1.0 release has been published and the independence of the module is established, consider changing things around for 1.1 or 2.0 or ...

I'm also not sure there would be a consensus — either before or after the move to a module — on deprecating the symbolic methods. (#> is a special case there; but by itself, the Dotty issue doesn't justify deprecating all of the symbolic methods at once.)

I perhaps shouldn't have slapped "help wanted" on this without being clear that I think some discussion is needed.

@joshlemer

This comment has been minimized.

Copy link
Member Author

joshlemer commented Oct 10, 2018

@SethTisue Maybe we should soften the requirements of this ticket to only ask for an addition of symbolic aliases.

@smarter

This comment has been minimized.

Copy link

smarter commented Oct 10, 2018

Whether or not the symbolic methods should be deprecated, I think that adding non-symbolic methods is a great improvement that I'd love to see in 2.13.

@lrytz

This comment has been minimized.

Copy link
Member

lrytz commented Oct 29, 2018

Would it even be necessary to remove method #>? Scala has separate namespaces for terms and types.

@smarter

This comment has been minimized.

Copy link

smarter commented Oct 29, 2018

No, it's not necessary, it just makes things less confusing.

@lrytz

This comment has been minimized.

Copy link
Member

lrytz commented Oct 31, 2018

The I agree with Seth to leave this as-is for now.

@SethTisue SethTisue removed the help wanted label Nov 6, 2018

@SethTisue SethTisue modified the milestones: 2.13.0-RC1, Backlog Nov 6, 2018

@SethTisue

This comment has been minimized.

Copy link
Member

SethTisue commented Nov 6, 2018

I've closed the PR for now, we could revisit if the #> stuff in Dotty moves forward

@SethTisue

This comment has been minimized.

Copy link
Member

SethTisue commented Nov 6, 2018

in other sys.process-related news, https://twitter.com/li_haoyi/status/1059650646616363008

@lihaoyi

This comment has been minimized.

Copy link

lihaoyi commented Nov 9, 2018

FWIW, Ammonite's filesystem/subprocess module has been extracted into a standalone, less-idiosyncratic library OS-Lib. OS-Lib's subprocess handling is heavily inspired by the Python subprocess API, and is both reasonably concise and operator-free:

// Start a long-lived python process which you can communicate with
val sub = os.proc("python", "-u", "-c", "while True: print(eval(raw_input()))")
            .spawn(cwd = wd)

// Sending some text to the subprocess
sub.stdin.write("1 + 2")
sub.stdin.writeLine("+ 4")
sub.stdin.flush()
sub.stdout.readLine() ==> "7"

// Sending some bytes to the subprocess
sub.stdin.write("1 * 2".getBytes)
sub.stdin.write("* 4\n".getBytes)
sub.stdin.flush()
sub.stdout.read() ==> '8'.toByte

sub.destroy()

// You can chain multiple subprocess' stdin/stdout together
val curl = os.proc("curl", "-L" , "https://git.io/fpfTs").spawn(stderr = os.Inherit)
val gzip = os.proc("gzip", "-n").spawn(stdin = curl.stdout)
val sha = os.proc("shasum", "-a", "256").spawn(stdin = gzip.stdout)
sha.stdout.trim ==> "acc142175fa520a1cb2be5b97cbbe9bea092e8bba3fe2e95afa645615908229e  -"

scala.sys.process hasn't been touched in more than a few years: the last significant changes were probably by mark harrah about a decade ago when it was still part of SBT. Apart from some trivial reformatting there hasn't been any real changes since Paul imported the whole thing from SBT into the scala/scala 8 (!) years ago. Honestly, at this point I don't think it's worth spending much effort trying to fix the API, and instead just focus on getting it out of scala/scala and start pointing people at better alternatives like the one above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment