Repository: bitfield/script
Pull request #2 (2019-06-09 14:03:45)

Add ListFiles function

SmilingNavern: I am not sure about this approach. Glob() and ReadDir() behaves differently in terms of output. So i provide relative path to files according to args.
SmilingNavern: I am going to fix this one then.
SmilingNavern: @bitfield i've updated my PR, thanks for such long delay. I've used ContainsAny and Readme parts from #5 and updated them a little. @abtin Check it please as well.
bitfield: Great! Could you check that you've addressed all the code review comments so far?
SmilingNavern: @bitfield i think i have addressed them all except one about Echo(), because i am not sure if this necessary and i am not sure about quality of Readme section, because i am bad in english and readme as well
SmilingNavern: Actually i thought that ListFiles maybe not a great name. It's more like ListDir(). But i didnt change that because i am not sure. The logic of code is more similiar to something that readdir() do.
bitfield: Yes, wherever you're creating a pipe from a string, `Echo()` is the thing to use. That means if we want to change the implementation from a `strings.Reader` to something else later, we can.

Don't worry about the Readme text, I'll copyedit that stuff and tweak the wording.

I'm not crazy about the ListFiles() name either, but I can't think of anything better at the moment. `Ls()` is too Unix-specific. `Files()` could be confused with what `Concat()` does—read the contents of the files, not just get the names.

This is where an example program really helps; it shows you how the name of the method will read when combined with others. Let's see if we can come up with an example program which gets a list of files and processes the names in some way.
SmilingNavern: Okay, i am not sure about name really. Let's stick to `ListFiles()` for now.

I will rewrite to Echo() and think about example.
SmilingNavern: I've fixed PR for using Echo() to create Pipe.
SmilingNavern: I've added example. But i think we need more function for files to provide more good examples. Maybe something like `RemoveFile()` or `Gzip()`/`Tar()`
SmilingNavern: Right now it's not really convenient to do something with this list of path except concating it to match something. I am not sure that `EachLine()` is convenient function for this type of task.
abtin: Actually I think an iterator function (e.g. Iter, ForEach) can be very
useful.

On Tue, Jul 9, 2019, 7:11 PM Alexander Danilov <notifications@github.com>
wrote:

> Right now it's not really convenient to do something with this list of
> path except concating it to match something. I am not sure that EachLine()
> is convenient function for this type of task.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/bitfield/script/pull/2?email_source=notifications&email_token=AADLYHFY5ANJRCTPM6CQBM3P6ULLPA5CNFSM4HWJPYNKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZRZQDY#issuecomment-509843471>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AADLYHEH6VNIGMQ5TKTKQTTP6ULLPANCNFSM4HWJPYNA>
> .
>

SmilingNavern: Yeap, i agree. But i am not sure how to implement it with Pipe concept. 
SmilingNavern: @bitfield hello, are there anything you want me to improve in this PR? I would be glad to hear any additional feedback.
bitfield: I'll review this as soon as I get a chance! It's looking very good!
bitfield: @SmilingNavern how are you getting along? Sorry I seem to have given you a very long list of changes to make here. It might actually be more efficient for me to make them myself on your branch, then we can agree any final steps that need to happen before merging. What do you think?

If you'd prefer to own the whole thing from start to finish, that's fine too—just let me know.
SmilingNavern: @bitfield i am going to finish this on weekend, so, please give me a time:) i was busy with work before.
SmilingNavern: I've fixed most of issues with my code based on your feedback. If there are any more of them feel free to update current branch and pull request, so we won't delay this PR any longer:)

thanks for your good feedback and sorry if my english isn't great:)
bitfield: Wonderful, thanks! I’ll review this as soon as I can.

Patch:
diff --git a/.gitignore b/.gitignore
index d5b7e86..6f80c42 100644
--- a/.gitignore
+++ b/.gitignore
@@ -5,3 +5,4 @@ examples/echo/echo
 examples/head/head
 examples/visitors/visitors
 .vscode/settings.json
+examples/ls/ls
diff --git a/README.md b/README.md
index c217d54..854a551 100644
--- a/README.md
+++ b/README.md
@@ -59,52 +59,54 @@ script.Args().Concat().Match("Error").First(10).AppendFile("/var/log/errors.txt"
 ```
 
 # Table of contents<!-- omit in toc -->
-- [What is `script`?](#What-is-script)
-- [How do I import it?](#How-do-I-import-it)
-- [What can I do with it?](#What-can-I-do-with-it)
-- [How does it work?](#How-does-it-work)
-- [Everything is a pipe](#Everything-is-a-pipe)
-- [What use is a pipe?](#What-use-is-a-pipe)
-- [Handling errors](#Handling-errors)
-- [Getting output](#Getting-output)
-- [Errors](#Errors)
-- [Closing pipes](#Closing-pipes)
-- [Why not just use shell?](#Why-not-just-use-shell)
-- [A real-world example](#A-real-world-example)
-- [Quick start: Unix equivalents](#Quick-start-Unix-equivalents)
-- [Sources, filters, and sinks](#Sources-filters-and-sinks)
-- [Sources](#Sources)
-	- [Args](#Args)
-	- [Echo](#Echo)
-	- [Exec](#Exec)
-		- [Exit status](#Exit-status)
-		- [Error output](#Error-output)
-	- [File](#File)
-	- [Stdin](#Stdin)
-- [Filters](#Filters)
-	- [Column](#Column)
-	- [Concat](#Concat)
-	- [EachLine](#EachLine)
-	- [Exec](#Exec-1)
-	- [First](#First)
-	- [Freq](#Freq)
-	- [Join](#Join)
-	- [Match](#Match)
-	- [MatchRegexp](#MatchRegexp)
-	- [Reject](#Reject)
-	- [RejectRegexp](#RejectRegexp)
-	- [Replace](#Replace)
-	- [ReplaceRegexp](#ReplaceRegexp)
-- [Sinks](#Sinks)
-	- [AppendFile](#AppendFile)
-	- [Bytes](#Bytes)
-	- [CountLines](#CountLines)
-	- [Read](#Read)
-	- [Stdout](#Stdout)
-	- [String](#String)
-	- [WriteFile](#WriteFile)
-- [Examples](#Examples)
-- [How can I contribute?](#How-can-I-contribute)
+- [What is `script`?](#what-is-script)
+- [How do I import it?](#how-do-i-import-it)
+- [What can I do with it?](#what-can-i-do-with-it)
+- [How does it work?](#how-does-it-work)
+- [Everything is a pipe](#everything-is-a-pipe)
+- [What use is a pipe?](#what-use-is-a-pipe)
+- [Handling errors](#handling-errors)
+- [Getting output](#getting-output)
+- [Errors](#errors)
+- [Closing pipes](#closing-pipes)
+- [Why not just use shell?](#why-not-just-use-shell)
+- [A real-world example](#a-real-world-example)
+- [Quick start: Unix equivalents](#quick-start-unix-equivalents)
+- [Sources, filters, and sinks](#sources-filters-and-sinks)
+- [Sources](#sources)
+	- [Args](#args)
+	- [Echo](#echo)
+	- [Exec](#exec)
+		- [Exit status](#exit-status)
+		- [Error output](#error-output)
+	- [File](#file)
+	- [ListFiles](#listfiles)
+	- [Slice](#slice)
+	- [Stdin](#stdin)
+- [Filters](#filters)
+	- [Column](#column)
+	- [Concat](#concat)
+	- [EachLine](#eachline)
+	- [Exec](#exec-1)
+	- [First](#first)
+	- [Freq](#freq)
+	- [Join](#join)
+	- [Match](#match)
+	- [MatchRegexp](#matchregexp)
+	- [Reject](#reject)
+	- [RejectRegexp](#rejectregexp)
+	- [Replace](#replace)
+	- [ReplaceRegexp](#replaceregexp)
+- [Sinks](#sinks)
+	- [AppendFile](#appendfile)
+	- [Bytes](#bytes)
+	- [CountLines](#countlines)
+	- [Read](#read)
+	- [Stdout](#stdout)
+	- [String](#string)
+	- [WriteFile](#writefile)
+- [Examples](#examples)
+- [How can I contribute?](#how-can-i-contribute)
 
 # How does it work?
 
@@ -279,6 +281,7 @@ If you're already familiar with shell scripting and the Unix toolset, here is a
 | `grep`             | [`Match()`](#match) / [`MatchRegexp()`](#matchregexp)         |
 | `grep -v`          | [`Reject()`](#reject) / [`RejectRegexp()`](#rejectregexp)     |
 | `head`             | [`First()`](#first)                                           |
+| `ls`               | [`ListFiles()`](#listfiles)                                   |
 | `sed`              | [`Replace()`](#replace) / [`ReplaceRegexp()`](#replaceregexp) |
 | `uniq -c`          | [`Freq()`](#freq)                                             |
 | `wc -l`            | [`CountLines()`](#countlines)                                 |
@@ -378,6 +381,34 @@ fmt.Println(output)
 // Output: contents of file
 ```
 
+## ListFiles
+
+`ListFiles()` lists files, like Unix [`ls`](examples/ls/main.go). It creates a pipe containing all files and directories matching the supplied path specification, one per line. This can be the name of a directory (`/path/to/dir`), the name of a file (`/path/to/file`), or a _glob_ (wildcard expression) conforming to the syntax accepted by [filepath.Match()](https://golang.org/pkg/path/filepath/#Match) (`/path/to/*`).
+
+```go
+p := script.ListFiles("/tmp/*.php")
+files, err := p.String()
+if err != nil {
+	log.Fatal(err)
+}
+fmt.Println("found suspicious PHP files in /tmp:")
+fmt.Println(files)
+```
+
+## Slice
+
+`Slice()` creates a pipe from a slice of strings, one per line.
+
+```go
+p := Slice([]string{"1", "2", "3"})
+output, err := p.String()
+fmt.Println(output)
+// Output:
+1
+2
+3
+```
+
 ## Stdin
 
 `Stdin()` creates a pipe which reads from the program's standard input.
@@ -703,6 +734,7 @@ Since `script` is designed to help you write system administration programs, a f
 * [head](examples/head/main.go)
 * [echo](examples/echo/main.go)
 * [visitors](examples/visitors/main.go)
+* [ls](examples/ls/main.go)
 
 [More examples would be welcome!](https://github.com/bitfield/script/pulls)
 
diff --git a/examples/ls/main.go b/examples/ls/main.go
new file mode 100644
index 0000000..69b0aec
--- /dev/null
+++ b/examples/ls/main.go
@@ -0,0 +1,19 @@
+package main
+
+import (
+	"os"
+	"regexp"
+
+	"github.com/bitfield/script"
+)
+
+// Like Unix `ls`, hide files starting with '.'
+var hideDotFiles = regexp.MustCompile(`[^/]*/\\.[^/]*$`)
+
+func main() {
+	listPath := "."
+	if len(os.Args) > 1 {
+		listPath = os.Args[1]
+	}
+	script.ListFiles(listPath).RejectRegexp(hideDotFiles).Stdout()
+}
diff --git a/go.mod b/go.mod
index a59ed19..24ac343 100644
--- a/go.mod
+++ b/go.mod
@@ -1,3 +1,5 @@
 module github.com/bitfield/script
 
 go 1.12
+
+require github.com/google/go-cmp v0.3.1
diff --git a/go.sum b/go.sum
new file mode 100644
index 0000000..a6ddb1d
--- /dev/null
+++ b/go.sum
@@ -0,0 +1,2 @@
+github.com/google/go-cmp v0.3.1 h1:Xye71clBPdm5HgqGwUkwhbynsUJZhDbS20FvLhQ2izg=
+github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
diff --git a/sources.go b/sources.go
index d218328..7de7920 100644
--- a/sources.go
+++ b/sources.go
@@ -1,7 +1,9 @@
 package script
 
 import (
+	"io/ioutil"
 	"os"
+	"path/filepath"
 	"strings"
 )
 
@@ -15,6 +17,36 @@ func Args() *Pipe {
 	return Echo(s.String())
 }
 
+// ListFiles creates a pipe containing the files and directories matching the
+// supplied path, one per line. The path may be a glob, conforming to
+// filepath.Match syntax.
+func ListFiles(path string) *Pipe {
+	if strings.ContainsAny(path, "[]^*?\\{}!") {
+		fileNames, err := filepath.Glob(path)
+		if err != nil {
+			return NewPipe().WithError(err)
+		}
+		return Slice(fileNames)
+	}
+	files, err := ioutil.ReadDir(path)
+	if err != nil {
+		// Check for the case where the path matches exactly one file
+		s, err := os.Stat(path)
+		if err != nil {
+			return NewPipe().WithError(err)
+		}
+		if !s.IsDir() {
+			return Echo(path)
+		}
+		return NewPipe().WithError(err)
+	}
+	fileNames := make([]string, len(files))
+	for i, f := range files {
+		fileNames[i] = filepath.Join(path, f.Name())
+	}
+	return Slice(fileNames)
+}
+
 // Echo returns a pipe containing the supplied string.
 func Echo(s string) *Pipe {
 	return NewPipe().WithReader(strings.NewReader(s))
@@ -39,6 +71,11 @@ func File(name string) *Pipe {
 	return p.WithReader(f)
 }
 
+// Slice returns a pipe containing each element of the supplied slice of strings, one per line.
+func Slice(s []string) *Pipe {
+	return Echo(strings.Join(s, "\n") + "\n")
+}
+
 // Stdin returns a pipe which reads from the program's standard input.
 func Stdin() *Pipe {
 	return NewPipe().WithReader(os.Stdin)


Test patch:
diff --git a/sources_test.go b/sources_test.go
index e089d32..0b7f908 100644
--- a/sources_test.go
+++ b/sources_test.go
@@ -1,29 +1,31 @@
 package script
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"os/exec"
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
 )
 
-func TestFile(t *testing.T) {
+func TestArgs(t *testing.T) {
 	t.Parallel()
-	wantRaw, _ := ioutil.ReadFile("testdata/test.txt") // ignoring error
-	want := string(wantRaw)
-	p := File("testdata/test.txt")
-	gotRaw, err := ioutil.ReadAll(p.Reader)
+	// dummy test to prove coverage
+	Args()
+	// now the real test
+	cmd := exec.Command(os.Args[0], "hello", "world")
+	cmd.Env = append(os.Environ(), "SCRIPT_TEST=args")
+	got, err := cmd.Output()
 	if err != nil {
 		t.Error(err)
 	}
-	got := string(gotRaw)
-	if got != want {
-		t.Errorf("want %q, got %q", want, got)
-	}
-	q := File("doesntexist")
-	if q.Error() == nil {
-		t.Errorf("want error status on opening non-existent file, but got nil")
+	want := "hello\nworld\n"
+	if string(got) != want {
+		t.Errorf("want %q, got %q", want, string(got))
 	}
+
 }
 
 func TestEcho(t *testing.T) {
@@ -76,38 +78,97 @@ func TestExec(t *testing.T) {
 	}
 }
 
-func TestStdin(t *testing.T) {
+func TestFile(t *testing.T) {
 	t.Parallel()
-	// dummy test to prove coverage
-	Stdin()
-	// now the real test
-	want := "hello world"
-	cmd := exec.Command(os.Args[0])
-	cmd.Env = append(os.Environ(), "SCRIPT_TEST=stdin")
-	cmd.Stdin = Echo(want).Reader
-	got, err := cmd.Output()
+	wantRaw, _ := ioutil.ReadFile("testdata/test.txt") // ignoring error
+	want := string(wantRaw)
+	p := File("testdata/test.txt")
+	gotRaw, err := ioutil.ReadAll(p.Reader)
 	if err != nil {
 		t.Error(err)
 	}
-	if string(got) != want {
-		t.Errorf("want %q, got %q", want, string(got))
+	got := string(gotRaw)
+	if got != want {
+		t.Errorf("want %q, got %q", want, got)
+	}
+	q := File("doesntexist")
+	if q.Error() == nil {
+		t.Errorf("want error status on opening non-existent file, but got nil")
 	}
 }
 
-func TestArgs(t *testing.T) {
+func TestListFilesMultipleFiles(t *testing.T) {
+	t.Parallel()
+	dir := "testdata/multiple_files"
+	want := fmt.Sprintf("%s/1.txt\n%s/2.txt\n%s/3.tar.zip\n", dir, dir, dir)
+	got, err := ListFiles(dir).String()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got != want {
+		t.Errorf("Want %q, got %q", want, got)
+	}
+}
+
+func TestListFilesNonexistent(t *testing.T) {
+	t.Parallel()
+	p := ListFiles("nonexistentpath")
+	if p.Error() == nil {
+		t.Error("want error status on listing non-existent path, but got nil")
+	}
+}
+
+func TestListFilesSingle(t *testing.T) {
+	t.Parallel()
+	got, err := ListFiles("testdata/multiple_files/1.txt").String()
+	if err != nil {
+		t.Fatal(err)
+	}
+	want := "testdata/multiple_files/1.txt"
+	if !cmp.Equal(want, got) {
+		t.Error(cmp.Diff(want, got))
+	}
+}
+
+func TestListFilesGlob(t *testing.T) {
+	t.Parallel()
+	dir := "testdata/multiple_files"
+	want := fmt.Sprintf("%s/1.txt\n%s/2.txt\n", dir, dir)
+	got, err := ListFiles("testdata/multi?le_files/*.txt").String()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if want != got {
+		t.Errorf("Want %q, got %q", want, got)
+	}
+}
+
+func TestSlice(t *testing.T) {
+	t.Parallel()
+	want := "1\n2\n3\n"
+	got, err := Slice([]string{"1", "2", "3"}).String()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !cmp.Equal(want, got) {
+		t.Error(cmp.Diff(want, got))
+	}
+}
+
+func TestStdin(t *testing.T) {
 	t.Parallel()
 	// dummy test to prove coverage
-	Args()
+	Stdin()
 	// now the real test
-	cmd := exec.Command(os.Args[0], "hello", "world")
-	cmd.Env = append(os.Environ(), "SCRIPT_TEST=args")
+	want := "hello world"
+	cmd := exec.Command(os.Args[0])
+	cmd.Env = append(os.Environ(), "SCRIPT_TEST=stdin")
+	cmd.Stdin = Echo(want).Reader
 	got, err := cmd.Output()
 	if err != nil {
 		t.Error(err)
 	}
-	want := "hello\nworld\n"
 	if string(got) != want {
 		t.Errorf("want %q, got %q", want, string(got))
 	}
-
 }