Skip to content

Implement Cmd.Clone() #35

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.

Sign up for GitHub

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

Merged
croqaz merged 3 commits into from
Jul 20, 2019
Merged

Implement Cmd.Clone() #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
12a6909
Implement Cmd.Clone
Jul 16, 2019
52d7b05
Updated the comment from Clone
Jul 16, 2019
83ee232
Removed obsolete lock
Jul 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,24 @@ func NewCmdOptions(options Options, name string, args ...string) *Cmd {
return out
}

// Clone clones a Cmd. All the options are transferred,
// but the internal state of the original object is lost.
// Cmd is one-use only, so if you need to re-start a Cmd,
// you need to Clone it.
Copy link
Member

@daniel-nichter daniel-nichter Jul 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Does the cloned Cmd reuse the original's Stdout and Stderr channels?

Copy link
Member Author

@croqaz croqaz Jul 18, 2019

Choose a reason for hiding this comment

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

Maybe I should document this another way, so it's more clear.

stdout, stderr and the status are dropped when cloning.
Only the public properties of the Cmd are transferred to the clone.
I'll add this ^ to the comment of the function.

Copy link
Member Author

@croqaz croqaz Jul 18, 2019

Choose a reason for hiding this comment

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

Oh wait, you mean the public Stdout and Stderr?
No, they shouldn't be transferred to the clone, because you want a fresh copy, ready to run from scratch. That's the point of the function.

Copy link
Member

@daniel-nichter daniel-nichter Jul 18, 2019

Choose a reason for hiding this comment

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

Ah yes, right: fresh Stdout and Stderr. 👍

func (c *Cmd) Clone() *Cmd {
clone := NewCmdOptions(
Options{
Buffered: c.buffered,
Streaming: c.Stdout != nil,
},
c.Name,
c.Args...,
)
clone.Dir = c.Dir
clone.Env = c.Env
return clone
}

// Start starts the command and immediately returns a channel that the caller
// can use to receive the final Status of the command when it ends. The caller
// can start the command and wait like,
Expand Down
20 changes: 20 additions & 0 deletions cmd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,26 @@ func TestCmdOK(t *testing.T) {
}
}

func TestCmdClone(t *testing.T) {
opt := cmd.Options{
Buffered: true,
}
c1 := cmd.NewCmdOptions(opt, "ls")
c1.Dir = "/tmp/"
c1.Env = []string{"YES=please"}
c2 := c1.Clone()

if c1.Name != c2.Name {
t.Errorf("got Name %s, expecting %s", c2.Name, c1.Name)
}
if c1.Dir != c2.Dir {
t.Errorf("got Dir %s, expecting %s", c2.Dir, c1.Dir)
}
if diffs := deep.Equal(c1.Env, c2.Env); diffs != nil {
t.Error(diffs)
}
}

func TestCmdNonzeroExit(t *testing.T) {
p := cmd.NewCmd("false")
gotStatus := <-p.Start()
Expand Down