Repository navigation
Support eezo new . scaffolds in the current directory - #338
what-the-functor wants to merge 3 commits into
Conversation
- Following functionality of other CLIs which scaffold in-place: Django, Phoenix, Rails, and React
a213f1c to
ad56a67
Compare
|
Just rebased on the |
rcardin
left a comment
rcardin
left a comment
There was a problem hiding this comment.
Thanks for this, the in place form is a good addition. The named path still produces an identical tree (I diffed the two scaffolds, and the 0.2.0 launcher against this one), and the gate is green on the merge with dev. Five inline comments, the symlinked .gitignore one is the heaviest. One note for the merge: the commit body has a dash bullet, which the repo's prose rule and the commit-msg hook on dev refuse, so the squash message needs a dashless body.
Exercised by hand, all passing unless commented: empty directory, directory names with a space and with ", an existing .gitignore with two of the four entries and no trailing newline, all four entries present, build.sbt plus src/ plus a dangling build.mill link, foreign content beside the scaffold, ./, "$PWD", ../shop, a symlinked working directory, a symlink argument pointing at ., the named form twice, no argument, a missing version file, and eezo --help.
* guard against appending to symlinked .gitignore * fail early if the .gitignore cannot be appended to * silence CD check * improve .gitignore documentation * disambiguate scaffolded path output and update its documentation
|
@rcardin thanks for your review. I've pushed fixes to address all of these. |
rcardin
left a comment
rcardin
left a comment
There was a problem hiding this comment.
Thanks, the five threads are answered and I exercised each one at the head. Two things the new created line and the new pre check introduce; both are one liners.
| EOF | ||
|
|
||
| echo "created ./$name (eezo $EEZO_VERSION, Scala $SCALA_VERSION, sbt $SBT_VERSION)" | ||
| echo "created $name at $(cd "$dir" && pwd) (eezo $EEZO_VERSION, Scala $SCALA_VERSION, sbt $SBT_VERSION)" |
There was a problem hiding this comment.
This cd is not CDPATH safe. With CDPATH=.:/tmp exported, a common idiom, bash's cd echoes the resolved path to stdout, so eezo new cd1 prints created cd1 at /…/cd1 and then /…/cd1 (eezo …) on a second line. Worse, with a CDPATH entry that holds a directory of the same name, the line names that other directory while the app was written to ./cd1. The old created ./$name had neither problem, and the in place form is immune because . bypasses CDPATH.
Simplest fix is to not cd at all: $PWD when $dir is ., $PWD/$name otherwise. If you prefer to keep the subshell, $(CDPATH= cd -- "$dir" > /dev/null && pwd) also works, and it removes the last unsilenced cd in the script.
| echo "eezo: .gitignore here is a symlink; new appends its entries only to a plain .gitignore" >&2 | ||
| exit 1 | ||
| } | ||
| [ ! -e .gitignore ] || { [ -f .gitignore ] && [ -w .gitignore ]; } || { |
There was a problem hiding this comment.
One more case for this check: it tests -w but not -r, and write_gitignore has to read the file too, with grep -qxF and tail -c1. On a write only .gitignore (printf 'target/\n' > .gitignore; chmod 200 .gitignore; eezo new .) the scaffold is written, every grep fails with Permission denied after the created line, all four entries are treated as missing, and the file ends up with target/ twice. Adding [ -r .gitignore ] next to -w closes it; the message and the sentence in docs/reference/cli.md already say "cannot append to", which covers reading as well.
|
@what-the-functor, I added some more comments 🙏 |
|
@rcardin, I've pushed a commit to address the CDPATH safety |
@what-the-functor, what about the other comment? |
|
@rcardin yes, the same commit adds the readability check. Forgot to mention that, sorry. |
Uh oh!
There was an error while loading. Please reload this page.