search

dsheets / tower

A Straightforward and Extensible Static Site Generator
7 stars 4 forks source link

Ignore files/directories starting with . or _ #23

Closed sgrove closed 9 years ago

sgrove commented 9 years ago

Closes #1

sgrove commented 9 years ago

If this push looks good, I'll rebase it into a single commit before merging.

dsheets commented 9 years ago

This looks pretty good. One thing it won't do that I didn't realize before is ignore paths in subdirectories that begin with '.' or '_' as foldp_paths will construct relative paths from the root. I think to handle that case, we need to change all_files to be filter_files or some such and pass another (??) predicate into foldp_paths. Alternately, all_files could return paths as lists of segments and then the single predicate could reject any path with a segment beginning with the hiding characters.

I think I favor this second approach but I haven't really thought a lot about the best way to do this. With segment lists, we'll need to List.fold_left (/) "" path in a couple places in TowerBuild.

dsheets commented 9 years ago

Oh, and a rebase would be most excellent.

sgrove commented 9 years ago

Do we want to ignore sub-directories beginning with . or _? I'll make the change, but not sure of the use case

dsheets commented 9 years ago

Subdirectories and files, I think. A lot of tools have subtree environments like git's .gitignore. What does Jekyll do?

sgrove commented 9 years ago

Looks like Jekyll (likely) also ignores them https://help.github.com/articles/files-that-start-with-an-underscore-are-missing/

Should any of this be accompanied by a test? I see right now the tests are visual rather than unit-test-library-driven

dsheets commented 9 years ago

Yes, the inclusion/exclusion of paths should probably be automatically tested. #14 tracks that. The sooner we move away from diff, the better. I've had good experiences with alcotest.

dsheets commented 9 years ago

This looks great! Only comment is List.exists/List.for_all is cheaper than List.filter + match because it can bail out when the predicate is false ASAP. As this code isn't performance critical in the slightest, I'm not too bothered by this.

I'll merge this tomorrow afternoon pending CLA discussion.