feat: add mk env for generating .env files#46
Conversation
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the mk env command to generate environment files from templates, including shell completions and tests. The review feedback points out that the implementation for the sh ports command is missing from the PR despite being included in the completions and test files. Additionally, there are suggestions to improve the test suite's robustness by ensuring proper file cleanup and making tests more self-contained.
| echo -e "${RED}✗${NC} mk env copies .env.example to .env (file not created)" | ||
| ((FAILED++)) | ||
| fi | ||
| rm -f .env |
There was a problem hiding this comment.
To make tests more robust and independent, each test should clean up all the files it creates. This test creates .env.example but only removes .env. The next test then implicitly depends on this leftover file. Please clean up .env.example here as well. You will then need to create it in the next test block.
| rm -f .env | |
| rm -f .env .env.example |
| rm -f .env | ||
|
|
||
| # Test: mk env errors when .env already exists | ||
| echo "existing" > .env |
There was a problem hiding this comment.
To make this test self-contained, it should create its own prerequisite files. Since the previous test should now be cleaning up .env.example, you need to create it here.
| echo "existing" > .env | |
| echo "DB_HOST=localhost" > .env.example | |
| echo "existing" > .env |
There was a problem hiding this comment.
Code Review
This pull request introduces a new mk env command to generate .env files from templates, including shell completions and comprehensive tests. Feedback focuses on the fragile argument parsing in lib/make.sh, which should be refactored into a loop to support flexible argument ordering. Additionally, error messages should be redirected to stderr. The PR also includes completions and tests for a sh ports command that appears to be missing its core implementation, leading to potential test failures and unrelated code changes that should be separated.
| if [[ "$1" == "from" ]]; then | ||
| if [[ -z "$2" ]]; then | ||
| echo "Usage: u7 mk env from <template> [to <output>]" | ||
| return 1 | ||
| fi | ||
| template="$2" | ||
| if [[ "$3" == "to" ]]; then | ||
| if [[ -z "$4" ]]; then | ||
| echo "Usage: u7 mk env from <template> to <output>" | ||
| return 1 | ||
| fi | ||
| output="$4" | ||
| fi | ||
| fi |
There was a problem hiding this comment.
The current argument parsing for mk env is not robust. It only handles a fixed order of arguments (from then to) and will ignore arguments if they don't appear in this exact sequence (e.g., u7 mk env to .env.local will be ignored). This doesn't match the flexibility suggested by the help text: env [from <template>] [to <output>].
To handle keyword arguments correctly, it's better to process them in a loop. This makes the command more user-friendly and less prone to errors. I've also updated the error messages to print to stderr, which is a best practice.
while [[ $# -gt 0 ]]; do
case "$1" in
from)
if [[ -z "$2" ]]; then
echo "Usage: u7 mk env from <template> [to <output>]" >&2
return 1
fi
template="$2"
shift 2
;;
to)
if [[ -z "$2" ]]; then
echo "Usage: u7 mk env [from <template>] to <output>" >&2
return 1
fi
output="$2"
shift 2
;;
*)
echo "Error: Unknown argument '$1' for 'mk env'" >&2
echo "Usage: u7 mk env [from <template>] [to <output>]" >&2
return 1
;;
esac
done| # Test: sh ports lists listening ports | ||
| result=$(u7 sh ports 2>&1) | ||
| if [[ $? -eq 0 ]]; then | ||
| echo -e "${GREEN}✓${NC} sh ports runs without error" | ||
| ((PASSED++)) | ||
| else | ||
| echo -e "${RED}✗${NC} sh ports runs without error" | ||
| echo " Got: $result" | ||
| ((FAILED++)) | ||
| fi | ||
|
|
||
| # Test: sh ports shows usage on invalid arg | ||
| result=$(u7 sh ports badarg 2>&1) | ||
| assert_contains "sh ports rejects invalid arg" "Usage:" "$result" | ||
|
|
||
| # Test: sh ports match requires pattern | ||
| result=$(u7 sh ports match 2>&1) | ||
| assert_contains "sh ports match requires pattern" "Usage:" "$result" | ||
|
|
||
| # Test: sh ports match filters output (may return nothing, just check it runs) | ||
| result=$(u7 sh ports match ssh 2>&1) | ||
| echo -e "${GREEN}✓${NC} sh ports match runs without crash" | ||
| ((PASSED++)) | ||
|
|
There was a problem hiding this comment.
These tests for sh ports are for a feature that is not implemented in this PR (the ports entity is missing from lib/show.sh). As a result, these tests will fail.
This feature is also unrelated to the main goal of the PR (mk env). It's a good practice to submit unrelated features in separate pull requests. Please remove these tests and other ports-related code from this branch.
| if [[ ! -f "$template" ]]; then | ||
| echo "Error: template '$template' not found" | ||
| return 1 | ||
| fi | ||
| if [[ -f "$output" ]]; then | ||
| echo "Error: '$output' already exists" | ||
| return 1 |
There was a problem hiding this comment.
It's a good practice to redirect error messages to standard error (stderr). This separates diagnostic messages from the script's main output, which is helpful for scripting and redirection.
| if [[ ! -f "$template" ]]; then | |
| echo "Error: template '$template' not found" | |
| return 1 | |
| fi | |
| if [[ -f "$output" ]]; then | |
| echo "Error: '$output' already exists" | |
| return 1 | |
| if [[ ! -f "$template" ]]; then | |
| echo "Error: template '$template' not found" >&2 | |
| return 1 | |
| fi | |
| if [[ -f "$output" ]]; then | |
| echo "Error: '$output' already exists" >&2 | |
| return 1 | |
| fi |
Greptile SummaryThis PR adds a
Confidence Score: 5/5Safe to merge — all remaining findings are P2 style/UX suggestions with no correctness impact. The core logic is correct: template existence is checked, output clobbering is prevented, dry-run is respected, and the test suite covers all documented scenarios. The three open comments are purely cosmetic (help text wording, missing success echo, incomplete tab completion after No files require special attention; Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A([u7 mk env ...]) --> B{first arg == 'from'?}
B -- yes --> C{second arg empty?}
C -- yes --> D[print Usage & return 1]
C -- no --> E[template = arg2]
E --> F{third arg == 'to'?}
F -- yes --> G{fourth arg empty?}
G -- yes --> H[print Usage & return 1]
G -- no --> I[output = arg4]
F -- no --> J[output = .env]
I --> K
J --> K
B -- no --> L[template = .env.example
output = .env]
L --> K{template file exists?}
K -- no --> M[Error: template not found
return 1]
K -- yes --> N{output file exists?}
N -- yes --> O[Error: output already exists
return 1]
N -- no --> P[_u7_exec cp template output]
P --> Q{dry-run mode?}
Q -- yes --> R[echo dry-run command]
Q -- no --> S[copy file to disk]
Prompt To Fix All With AIThis is a comment left during a code review.
Path: lib/make.sh
Line: 235
Comment:
**Misleading syntax in help text**
The current help text implies `[to <output>]` can be used independently of `[from <template>]`:
```
env [from <template>] [to <output>]
```
However, in the implementation `to <output>` is only parsed when `from <template>` is already present — calling `u7 mk env to custom.env` silently ignores `to custom.env` and falls back to the default `.env` output. The help text should reflect the actual nested-optional structure:
```suggestion
env [from <template> [to <output>]] Generate .env from .env.example or a custom template
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: lib/completions.sh
Line: 48
Comment:
**No file-path completion after `from` keyword**
The current completion only suggests `from` as the first argument after `env`. Once the user types `u7 mk env from <TAB>`, there is no further completion, so the template file path cannot be tab-completed. Adding a `_filedir` fallback when no keyword matches — similar to how `copy|link` already does — would improve usability:
```suggestion
env) COMPREPLY=($(compgen -W "from" -- "$cur")); [[ -z "$COMPREPLY" ]] && _filedir ;;
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: lib/make.sh
Line: 200
Comment:
**No success feedback message**
The `cp` command produces no output on success, leaving the user with no confirmation that `.env` was created. Other similar entities (e.g., `copy`, `template`) print a success line. Adding one here would keep the UX consistent:
```suggestion
_u7_exec cp "$template" "$output" && echo "Created '$output' from '$template'"
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat: add mk env entity for generating ...." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| archive <output> from <files...> Create archive from <files...> to <output> | ||
| clone <repo> [to <directory>] Git clone a repository | ||
| template <python|node|bash|web> <name> Scaffold a project structure | ||
| env [from <template>] [to <output>] Generate .env from .env.example or a custom template |
There was a problem hiding this comment.
The current help text implies [to <output>] can be used independently of [from <template>]:
env [from <template>] [to <output>]
However, in the implementation to <output> is only parsed when from <template> is already present — calling u7 mk env to custom.env silently ignores to custom.env and falls back to the default .env output. The help text should reflect the actual nested-optional structure:
| env [from <template>] [to <output>] Generate .env from .env.example or a custom template | |
| env [from <template> [to <output>]] Generate .env from .env.example or a custom template |
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/make.sh
Line: 235
Comment:
**Misleading syntax in help text**
The current help text implies `[to <output>]` can be used independently of `[from <template>]`:
```
env [from <template>] [to <output>]
```
However, in the implementation `to <output>` is only parsed when `from <template>` is already present — calling `u7 mk env to custom.env` silently ignores `to custom.env` and falls back to the default `.env` output. The help text should reflect the actual nested-optional structure:
```suggestion
env [from <template> [to <output>]] Generate .env from .env.example or a custom template
```
How can I resolve this? If you propose a fix, please make it concise.| make|mk) | ||
| case "$entity" in | ||
| copy|link) _filedir ;; | ||
| env) COMPREPLY=($(compgen -W "from" -- "$cur")) ;; |
There was a problem hiding this comment.
from keyword
The current completion only suggests from as the first argument after env. Once the user types u7 mk env from <TAB>, there is no further completion, so the template file path cannot be tab-completed. Adding a _filedir fallback when no keyword matches — similar to how copy|link already does — would improve usability:
| env) COMPREPLY=($(compgen -W "from" -- "$cur")) ;; | |
| env) COMPREPLY=($(compgen -W "from" -- "$cur")); [[ -z "$COMPREPLY" ]] && _filedir ;; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/completions.sh
Line: 48
Comment:
**No file-path completion after `from` keyword**
The current completion only suggests `from` as the first argument after `env`. Once the user types `u7 mk env from <TAB>`, there is no further completion, so the template file path cannot be tab-completed. Adding a `_filedir` fallback when no keyword matches — similar to how `copy|link` already does — would improve usability:
```suggestion
env) COMPREPLY=($(compgen -W "from" -- "$cur")); [[ -z "$COMPREPLY" ]] && _filedir ;;
```
How can I resolve this? If you propose a fix, please make it concise.| echo "Error: '$output' already exists" | ||
| return 1 | ||
| fi | ||
| _u7_exec cp "$template" "$output" |
There was a problem hiding this comment.
The cp command produces no output on success, leaving the user with no confirmation that .env was created. Other similar entities (e.g., copy, template) print a success line. Adding one here would keep the UX consistent:
| _u7_exec cp "$template" "$output" | |
| _u7_exec cp "$template" "$output" && echo "Created '$output' from '$template'" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/make.sh
Line: 200
Comment:
**No success feedback message**
The `cp` command produces no output on success, leaving the user with no confirmation that `.env` was created. Other similar entities (e.g., `copy`, `template`) print a success line. Adding one here would keep the UX consistent:
```suggestion
_u7_exec cp "$template" "$output" && echo "Created '$output' from '$template'"
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
enventity tou7 mkfor generating.envfiles from.env.exampleor custom templatesu7 mk envcopies.env.exampleto.envwith existence checksu7 mk env from <template> [to <output>]supports custom template and output pathsTest plan
u7 mk envcopies.env.exampleto.env.envalready existsu7 mk env from <template>uses custom templateu7 mk env from <template> to <output>uses custom output path