Implement a mode to exclude directories defined with arguments#36
Implement a mode to exclude directories defined with arguments#36ishkawa merged 4 commits intoishkawa:masterfrom
Conversation
ishkawa
left a comment
ishkawa
left a comment
There was a problem hiding this comment.
Basically, looks good to me!
I've left some comments, please confirm them.
| } | ||
| } catch { | ||
| print("error: invalid arguments", to: &standardError) | ||
| print("usage: dikitgen <path to source code directory> [--exclude <subpaths to exclude>...]", to: &standardError) |
There was a problem hiding this comment.
Good documentation 👍
This part ofmode(from:) function seems to assume --exclude DIR appears only once .
If it is right, ... doesn't express its behavior correctly.
There was a problem hiding this comment.
It accepts multiple directories (like --exclude Example Tests)
What stlye do you think is appropriate to represent it?
There was a problem hiding this comment.
Oh, I misunderstood this 🙇
I think it's better to specify --exclude for each exclusions like --exclude DIR1 --exclude DIR2, because the meaning is clearer especially when we have more options in the future.
There was a problem hiding this comment.
I got it. I’m going to fix to the style!
|
|
||
| private func files(atPath path: String) -> [File] { | ||
| private func files(atPath path: String, excluding exclusions: [String]) -> [File] { | ||
| let exclusionsHavingSlash = exclusions.map { $0.last == "/" ? $0 : $0 + "/" } |
There was a problem hiding this comment.
nit: Shadowing exclusions prevents from accessing original exclusion accidentally.
let exclusions = exclusions.map { $0.last == "/" ? $0 : $0 + "/" }
|
👍 I'll consider how we test CLI options later. |
2 participants
resolve #35
I'm sorry but I don't find how to test the command.