Change open(cmd) to return Process instead of Tuple #12807

vtjnash · 2015-08-26T02:45:09Z

fixes #9659 by changing open(Cmd) to only return a Process, and defining close(Process) to mean close(Process.in) && close(Process.out). this is a breaking change, since i know of no way to sanely provide a deprecation for this (I would prefer not to define iteration over a Process object as returning the old tuple of (io, process))

StefanKarpinski · 2015-08-26T03:11:01Z

If I'm groking this change correctly, I think it's an improvement. Can you give an example of the new usage as opposed to the old one (which I never cared for)?

vtjnash · 2015-08-26T03:23:41Z

just take a look at the PR diff, since most of it is to deal with the impact of the switch on usages in this repo

StefanKarpinski · 2015-08-26T04:43:34Z

Wouldn't it be fairly straightforward to add deprecated methods to iterate a process object?

vtjnash · 2015-08-26T05:06:46Z

somewhat, it's just really strange to define the iteration to first return either .in or .out (depending on a flag set during construction), then return the object itself.

StefanKarpinski · 2015-08-26T12:37:45Z

Agreed, but it's just for deprecation purposes, it's not like we'd be advocating that API (quite the opposite).

vtjnash · 2015-08-26T23:05:26Z

alright, I think those methods should cover the most common usages of the previous definition

mbaz · 2015-09-03T23:08:32Z

Is there any chance of having this merged before 0.4 is released? I'm almost ready to release a new Gaston version to go along with 0.4, but this issue is holding it up.

tkelman · 2015-09-04T05:29:53Z

Unlikely unless this gets changed to not be breaking.

vtjnash · 2015-09-12T23:27:26Z

rebased. should I merge this and see whether it plays out as a good change, or perhaps suggests an even better design in the future?

StefanKarpinski · 2015-09-14T17:39:36Z

I think it's too late for this to get into 0.4, unfortunately.

vtjnash · 2015-09-14T17:49:07Z

The question was for master, not 0.4

StefanKarpinski · 2015-09-14T17:52:05Z

Oh, right, that's a different story

tkelman · 2015-09-14T18:14:34Z

base/deprecated.jl

@@ -770,6 +770,16 @@ end
 const FloatingPoint = AbstractFloat
 export FloatingPoint

+# 12807


move to new 0.5 section along with AsyncStream

vtjnash · 2015-09-14T19:27:12Z

this is failing due to a (subtle but common) bug in the test of our IO framework. anyone care to take the shot at guessing?

tkelman · 2016-02-28T07:48:09Z

Clearly nobody guessed. Does it still apply?

Also, does this implement the

a process object should inherit from stream and redirect reads and writes to the appropriate stdin/stdout stream

portion of the suggested change? I'm not seeing that.

tkelman · 2016-09-14T20:20:42Z

does this need doc changes? along with a rebase obviously

JeffBezanson · 2017-01-06T22:01:24Z

Bump.

StefanKarpinski · 2017-01-08T17:01:06Z

@vtjnash – I'm pretty overloaded right now. Any chance you could dust this off on move it forward?

vtjnash · 2017-04-24T22:17:45Z

dusted.

🙂

tkelman · 2017-04-24T22:18:58Z

base/deprecated.jl

@@ -1307,6 +1307,15 @@ end
     end
 end

+# 12807


amitmurthy · 2017-04-25T04:58:55Z

Use variable name pobj instead of io everywhere as it is the process object and not the io handle being returned.

vtjnash · 2017-04-25T05:09:27Z

It's both: the process objects are also AbstractPipe objects. Generally I tried to preserve whichever name seemed to better reflect how it was to be used.

…lue) This reverts commit 8ffdfc2, and fixes it a bit too :) fix #9659

tkelman

deprecations need to move to 0.7 section

deprecations moved

missed from #12807, only happens when running in mintty

amitmurthy · 2017-05-26T08:34:57Z

Docstring here

julia/base/process.jl

Line 571 in 9e3318c

    
           Start running `command` asynchronously, and return a tuple `(stream,process)`.  If `mode` is

needs to be updated.

JuliaLang/julia#12807

rened · 2018-03-26T14:51:13Z

Just fyi, the above docstring is still the old one, needs updating.

bicycle1885 · 2018-03-26T16:30:06Z

This is the pull request I made long before to fix the docstring: #25563.

vtjnash added the breaking This change will break code label Aug 26, 2015

vtjnash mentioned this pull request Aug 26, 2015

process and pipe cleanup #12739

Merged

vtjnash force-pushed the jn/open_cmd_process branch from 955fab4 to 422521d Compare August 26, 2015 23:04

vtjnash force-pushed the jn/open_cmd_process branch from 422521d to db79a03 Compare August 27, 2015 00:23

vtjnash force-pushed the jn/open_cmd_process branch from db79a03 to 0712f9c Compare September 12, 2015 23:26

tkelman reviewed Sep 14, 2015
View reviewed changes

tkelman mentioned this pull request Feb 28, 2016

use of open() results or filehandles #9659

Closed

StefanKarpinski mentioned this pull request Jan 2, 2017

things we should deprecate, 0.6 edition #19598

Closed

22 tasks

vtjnash force-pushed the jn/open_cmd_process branch from 0712f9c to 06a2be1 Compare April 24, 2017 22:16

tkelman reviewed Apr 24, 2017

View reviewed changes

base/deprecated.jl Outdated

@@ -1307,6 +1307,15 @@ end

end

end

# 12807

Copy link

Contributor

tkelman Apr 24, 2017

Choose a reason for hiding this comment

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

post 0.6

StefanKarpinski changed the title ~~Change open(cmd) to return Process instead of Tuple~~ POST 0.6: Change open(cmd) to return Process instead of Tuple Apr 25, 2017

change open(cmd) to return a Process object (without a separate IO va…

94acb6f

…lue) This reverts commit 8ffdfc2, and fixes it a bit too :) fix #9659

vtjnash force-pushed the jn/open_cmd_process branch from 06a2be1 to 85f3fc7 Compare May 2, 2017 20:21

tkelman previously requested changes May 2, 2017

View reviewed changes

StefanKarpinski changed the title ~~POST 0.6: Change open(cmd) to return Process instead of Tuple~~ Change open(cmd) to return Process instead of Tuple May 2, 2017

add deprecation mechanism for (io,p) = open(cmd)

505dec5

vtjnash force-pushed the jn/open_cmd_process branch from 85f3fc7 to 505dec5 Compare May 2, 2017 20:55

vtjnash merged commit 0cc2b96 into master May 8, 2017

vtjnash deleted the jn/open_cmd_process branch May 8, 2017 02:31

tkelman added the needs news A NEWS entry is required for this change label May 8, 2017

Sacha0 mentioned this pull request May 12, 2017

Make sure NEWS.md lists all 0.6 deprecations and breaking changes #21475

Closed

tkelman added a commit that referenced this pull request May 13, 2017

Fix deprecated indexing of open call in windows displaysize

1e81c1c

missed from #12807, only happens when running in mintty

tkelman mentioned this pull request May 13, 2017

Fix deprecated indexing of open call in windows displaysize #21862

Merged

musm mentioned this pull request May 14, 2017

open(cmd) to return Process instead of Tuple JuliaLang/Compat.jl#357

Closed

tkelman added a commit that referenced this pull request May 14, 2017

Fix deprecated indexing of open call in windows displaysize

cba1906

missed from #12807, only happens when running in mintty

tkelman added a commit that referenced this pull request May 14, 2017

Fix deprecated indexing of open call in windows displaysize

233435b

missed from #12807, only happens when running in mintty

omus added a commit to JuliaTesting/Mocking.jl that referenced this pull request Oct 31, 2017

open(cmd) now returns Process instead of Tuple

23f6ebe

JuliaLang/julia#12807

KristofferC removed the needs news A NEWS entry is required for this change label Nov 13, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change open(cmd) to return Process instead of Tuple #12807

Change open(cmd) to return Process instead of Tuple #12807

vtjnash commented Aug 26, 2015

StefanKarpinski commented Aug 26, 2015

vtjnash commented Aug 26, 2015

StefanKarpinski commented Aug 26, 2015

vtjnash commented Aug 26, 2015

StefanKarpinski commented Aug 26, 2015

vtjnash commented Aug 26, 2015

mbaz commented Sep 3, 2015

tkelman commented Sep 4, 2015

vtjnash commented Sep 12, 2015

StefanKarpinski commented Sep 14, 2015

vtjnash commented Sep 14, 2015

StefanKarpinski commented Sep 14, 2015

tkelman Sep 14, 2015

vtjnash commented Sep 14, 2015

tkelman commented Feb 28, 2016

tkelman commented Sep 14, 2016

JeffBezanson commented Jan 6, 2017

StefanKarpinski commented Jan 8, 2017

vtjnash commented Apr 24, 2017

tkelman Apr 24, 2017

amitmurthy commented Apr 25, 2017

vtjnash commented Apr 25, 2017

tkelman left a comment

amitmurthy commented May 26, 2017

rened commented Mar 26, 2018

bicycle1885 commented Mar 26, 2018

@@ @@ -1307,6 +1307,15 @@ end @@
                    end
               end
+              # 12807

Change open(cmd) to return Process instead of Tuple #12807

Change open(cmd) to return Process instead of Tuple #12807

Conversation

vtjnash commented Aug 26, 2015

StefanKarpinski commented Aug 26, 2015

vtjnash commented Aug 26, 2015

StefanKarpinski commented Aug 26, 2015

vtjnash commented Aug 26, 2015

StefanKarpinski commented Aug 26, 2015

vtjnash commented Aug 26, 2015

mbaz commented Sep 3, 2015

tkelman commented Sep 4, 2015

vtjnash commented Sep 12, 2015

StefanKarpinski commented Sep 14, 2015

vtjnash commented Sep 14, 2015

StefanKarpinski commented Sep 14, 2015

tkelman Sep 14, 2015

Choose a reason for hiding this comment

vtjnash commented Sep 14, 2015

tkelman commented Feb 28, 2016

tkelman commented Sep 14, 2016

JeffBezanson commented Jan 6, 2017

StefanKarpinski commented Jan 8, 2017

vtjnash commented Apr 24, 2017

tkelman Apr 24, 2017

Choose a reason for hiding this comment

amitmurthy commented Apr 25, 2017

vtjnash commented Apr 25, 2017

tkelman left a comment

Choose a reason for hiding this comment

amitmurthy commented May 26, 2017

rened commented Mar 26, 2018

bicycle1885 commented Mar 26, 2018