Feature/optional files

[chen-factual]

For #179

@amalloy
Please review? I basically guessed where things are supposed to go (changes in core.clj and parser.clj), I expect you to ding me pretty hard on organization.

I also didn't test thoroughly beyond basic functionality (i.e. up-tree, down-tree, etc etc). If you can give some direction there that'd be cool.

Test Drakefile, one missing file, two existing ones.

%test <- ?missing, ?existing-1, existing-2
  echo "1" $INPUT0
  echo "2" $INPUT1
  echo "3" $INPUT2
  echo "ALL" $INPUTS
  echo "N" $INPUTN

Shell

~/code/drake $ ls sandbox/
existing-1  existing-2
~/code/drake $ lein run --base sandbox/ %test
Compiling drake.parser
The following steps will be run, in order:
  1: %test <- ?sandbox/missing, ?sandbox/existing-1, sandbox/existing-2 [via tag]
Confirm? [y/n] y
Running 1 steps with concurrence of 1...

--- 0. Running (via tag): %test <- ?sandbox/missing, ?sandbox/existing-1, sandbox/existing-2
1
2 /home/chen/code/drake/sandbox/existing-1
3 /home/chen/code/drake/sandbox/existing-2
ALL /home/chen/code/drake/sandbox/existing-1 /home/chen/code/drake/sandbox/existing-2
N 3
--- 0: %test <- ?sandbox/missing, ?sandbox/existing-1, sandbox/existing-2 -> done in 0.06s
Done (1 steps run).

[chen-factual]

I can see the case for generating these structures at parse time (similar to the "two-copies" approach Artem suggested in his comment about this feature), but honestly the parser code confuses me so I did this here.

We'd save a couple extra checks for existence, but the change will be more extensive as inputs will now be this map instead of just a string, and everything has to handle that. It's your call.

[amalloy]

IMO the parser already does too much stuff unrelated to parsing. It shouldn't be reading the filesystem to decide what is meant by the stuff it parses. So where you've put this looks right to me.

[amalloy]

I think this should return a map like {:existing (...) :empty (...)}, so that it will be clearer at the usage site what's going on.

[amalloy]

Overall looks pretty good to me.

[amalloy]

@chen-factual What do you think about a function that wraps functions so that they correctly handle optional inputs? Like instead of making normalize-filename-for-run and similar functions call parser/modify-filename themselves, you could just rename the old definition to normalize-filename-for-run*, and then (def normalize-filename-for-run (wrap-optional-handling normalize-filename-for-run*)) or whatever.

[chen-factual]

That's a pretty awesome idea. I'll go ahead and pop that in there when I make my modifications for your review comments.

I'll probably do this late night tonight.

[chen-factual]

You know, I thought about this some more. Why would you ever NOT want a function to correctly handle optional files? Why even have a normalize-filename-for-run* that fails for optional files?

Also in the case of functions like parser/add-prefix there there are other arguments besides a file name, this wrapping breaks down as we have no way of telling which argument is the file name.

I'm open to improving this in another way if you have any other ideas though.

[amalloy]

Applied as b18cc24.