MVP for find and xargs

[drwilly]

No description provided.

[andychu]

Thanks! I'm about to release 0.6.0 and write a blog post about it. After that I will clone this, run the tests, and look at the code.

[andychu]

I checked out this PR, here are my initial comments.

Please put a license at the top of the files you wrote, so there's no question of ownership. If they're mixed, you can put both our names (although it doesn't matter that much). I need to go back and to that for some Oil files too.

# Copyright 2016 Wilke. All rights reserved.
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#   http://www.apache.org/licenses/LICENSE-2.0

• ./xargs.py_test has a weird name. How about xargs-test.sh ? I wasn't sure if it was a Python file or a shell file.
• Likewise xargs/testdata is conventional for data files
• It should mkdir -p _tmp at the beginning

./xargs.py_test: 18: ./xargs.py_test: cannot create _tmp/delimiter_stdout_actual: Directory nonexistent
./xargs.py_test: 17: ./xargs.py_test: cannot create _tmp/delimiter_stdout_expected: Directory nonexistent

---

• ditto, please rename to find-test.sh and testdata to be consistent with the rest of the repo.
  • I don't want to do this myself because it may cause problems for merges later. I think it's easier if you do it.

---

I didn't look at the code that carefully, but I think those changes are enough for the first pass. That is, just renaming and adding licenses.

And then you can continue working in your branch until whenever you feel the next natural PR is.

I will start adding some options to our own shell scripts to dogfood these find and xargs implementations in the meantime. That way they can get some testing before we release them.

As mentioned, eventually we can hook it up to bin/oil.py, and the type checker, but it doesn't have to be done now.

Thanks a lot for doing this!

[drwilly]

* `./xargs.py_test` has a weird name.  How about `xargs-test.sh` ?  I wasn't sure if it was a Python file or a shell file.

* Likewise `xargs/testdata` is conventional for data files

Shouldn't matter. It's an executable file, that is all that needs to be known.
Not to open a discussion, but the thought process is this:
xargs.py is the main program.
xargs.py_test is the test for said program.
xargs.py_test_input is the input for said test
I'm a simple person.

So, xargs-test.sh?

[andychu]

Yeah I see why you did it that way, but xargs-test.sh and testdata follows the style of the rest of the repo -- e.g. see ls */*-test.sh.

(Most test drivers for the shell are in test/*.sh, but that style is what we use for shell tests for standalone tools.)

[andychu]

@drwilly I disabled the lint checks for these two dirs, so you don't have to worry about it for now.

6a120df

At some point it will become worthwhile to enable them, as well as the type checks, but I'll let you decide when.

[drwilly]

Sooo … ?

[andychu]

Oh you pushed, but didn't respond... After you make some changes, please respond so I know to look again. I had this problem with other PRs, so I documented it here:

"Pull Requests"

https://github.com/oilshell/oil/wiki/Contributing

[andychu]

Oh it looks like you added licenses to xargs but not find ?

[drwilly]

Huh? It's right there.

[andychu]

Only 1 file looks like it has a license. But it's OK I will pull it and add them, and make some other changes. Make sure that you can merge back, so the next merge is easy.

[andychu]

@drwilly OK I made small changes, please merge:

8441c65

Now you can go work in parallel while I test it out more :)