fix: implement code review findings - fix typos, add verbose flag to dotenv, improve error handling (#279)

* Initial plan

* Fix typos, add license header, standardize paths, improve extractAppName, add verbose to dotenv

Co-authored-by: panz3r <1754457+panz3r@users.noreply.github.com>

* Add verbose flag tests to icons, splash, and dotenv commands

Co-authored-by: panz3r <1754457+panz3r@users.noreply.github.com>

* Add unit tests for extractAppName utility function

Co-authored-by: panz3r <1754457+panz3r@users.noreply.github.com>

* refactor: update `extractAppName` to use async file reading and return `undefined`

* test: update `extractAppName` tests to use async/await and check returned `undefined`

* test: update `extractAppName` tests titles

* docs: update 001-CODE-REVIEW-FINDINGS.md with completion status

Co-authored-by: panz3r <1754457+panz3r@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: panz3r <1754457+panz3r@users.noreply.github.com>

Author: Copilot

docs/001-CODE-REVIEW-FINDINGS.md

@@ -1,22 +1,44 @@
 # Code Review Findings
 
-**Status:** Pending  
+**Status:** Partially Completed  
 **Created:** 2026-01-05  
+**Updated:** 2026-01-06  
 **Target:** Improvement recommendations from a code review, prioritized for the "Test Developer" agent.
 
+## Completion Summary
+
+**Completed Items (6/11):**
+- ✅ Fix typo in error messages (icons.ts, splash.ts)
+- ✅ Add missing license header to constants.ts
+- ✅ Standardize path prefixes in splash.ts
+- ✅ Improve extractAppName error handling
+- ✅ Add verbose flag to dotenv command
+- ✅ Add tests for verbose output (icons, splash, dotenv)
+- ✅ Add unit tests for extractAppName utility
+
+**Pending Items (5/11):**
+- ⏳ Add tests for malformed input images
+- ⏳ Extract base command class
+- ⏳ Add type definitions for icon sizes
+- ⏳ Improve error collection in commands
+- ⏳ Add tests for utility files (file-utils, color.utils)
+
 ---
 
 ## Test Coverage Improvements (Priority: Medium)
 
-### 1. Add Tests for Verbose Output
+### 1. Add Tests for Verbose Output ✅ COMPLETED
 
-The `-v` flag behavior is not explicitly tested in any command.
+**Status:** ✅ Completed in PR #XX
 
-**Files to update:**
-- `test/commands/icons.test.ts`
-- `test/commands/splash.test.ts`
+The `-v` flag behavior is now tested in all commands.
 
-**Test cases to add:**
+**Files updated:**
+- ✅ `test/commands/icons.test.ts` - Added verbose flag test
+- ✅ `test/commands/splash.test.ts` - Added verbose flag test
+- ✅ `test/commands/dotenv.test.ts` - Added verbose flag test
+
+**Test cases added:**
 ```typescript
 it('runs icons with verbose flag and shows detailed output', async () => {
   const {stdout} = await runCommand(['icons', '--appName', 'test', '-v'])
@@ -26,7 +48,9 @@ it('runs icons with verbose flag and shows detailed output', async () => {
 })
 ```
 
-### 2. Add Tests for Malformed Input Images
+### 2. Add Tests for Malformed Input Images ⏳ PENDING
+
+**Status:** ⏳ Not yet implemented
 
 No tests verify behavior with corrupt or wrong-format files.
 
@@ -41,16 +65,20 @@ it('handles corrupt image file gracefully', async () => {
 })
 ```
 
-### 3. Add Unit Tests for `extractAppName` Utility
+### 3. Add Unit Tests for `extractAppName` Utility ✅ COMPLETED
+
+**Status:** ✅ Completed in PR #XX
 
-**File to create:** `test/utils/app.utils.test.ts`
+**File created:** `test/app.utils.test.ts`
 
-**Test cases:**
-- Valid `app.json` with `name` property
-- Missing `app.json` file
-- Invalid JSON in `app.json`
-- Valid JSON but missing `name` property
-- Empty `name` property
+**Test cases implemented:**
+- ✅ Valid `app.json` with `name` property
+- ✅ Missing `app.json` file
+- ✅ Invalid JSON in `app.json`
+- ✅ Valid JSON but missing `name` property
+- ✅ Empty `name` property
+- ✅ Whitespace-only `name` property
+- ✅ Non-string `name` property
 
 ```typescript
 import {expect} from 'chai'
@@ -77,29 +105,33 @@ describe('extractAppName', () => {
     expect(extractAppName()).to.be.null
   })
 
-  it('returns undefined when name property is missing', () => {
+  it('returns null when name property is missing', () => {
     fs.writeFileSync('app.json', JSON.stringify({version: '1.0.0'}))
-    expect(extractAppName()).to.be.undefined
+    expect(extractAppName()).to.be.null
   })
 })
 ```
 
-### 4. Add Tests for `dotenv` Verbose Flag
+### 4. Add Tests for `dotenv` Verbose Flag ✅ COMPLETED
 
-Once verbose flag is added to `dotenv` command, add corresponding tests.
+**Status:** ✅ Completed in PR #XX
+
+Verbose flag was added to `dotenv` command and corresponding tests were added.
 
 ---
 
 ## Code Fixes Required Before Testing
 
-### 1. Fix Typo in Error Messages (Priority: Low)
+### 1. Fix Typo in Error Messages ✅ COMPLETED
+
+**Status:** ✅ Completed in PR #XX
 
 **Files:** `src/commands/icons.ts`, `src/commands/splash.ts`
 
-Change `"retrive"` to `"retrieve"` in error messages.
+Changed `"retrive"` to `"retrieve"` in error messages.
 
-**Location in icons.ts:** Line 68
-**Location in splash.ts:** Line 66
+**Location in icons.ts:** Line 65
+**Location in splash.ts:** Line 64
 
 ```typescript
 // Before:
@@ -109,11 +141,13 @@ this.error(`${red('✘')} Failed to retrive ${cyan('appName')} value...`)
 this.error(`${red('✘')} Failed to retrieve ${cyan('appName')} value...`)
 ```
 
-### 2. Add Verbose Flag to `dotenv` Command (Priority: Medium)
+### 2. Add Verbose Flag to `dotenv` Command ✅ COMPLETED
+
+**Status:** ✅ Completed in PR #XX
 
 **File:** `src/commands/dotenv.ts`
 
-Add verbose flag to match `icons` and `splash` commands for consistency.
+Added verbose flag to match `icons` and `splash` commands for consistency.
 
 ```typescript
 static override flags = {
@@ -126,11 +160,13 @@ static override flags = {
 }
 ```
 
-### 3. Add Missing License Header (Priority: Low)
+### 3. Add Missing License Header ✅ COMPLETED
+
+**Status:** ✅ Completed in PR #XX
 
 **File:** `src/constants.ts`
 
-Add MPL-2.0 license header to match other source files:
+Added MPL-2.0 license header to match other source files:
 
 ```typescript
 /*
@@ -142,31 +178,38 @@ Add MPL-2.0 license header to match other source files:
  */
 ```
 
-### 4. Standardize Path Prefixes (Priority: Low)
+### 4. Standardize Path Prefixes ✅ COMPLETED
+
+**Status:** ✅ Completed in PR #XX
 
 **Files:** `src/commands/icons.ts`, `src/commands/splash.ts`
 
-Inconsistent use of `./` prefix in paths:
+Removed inconsistent use of `./` prefix in paths.
 
 **In icons.ts (no prefix):**
 ```typescript
 this.generateAndroidIcons(args.file, 'android/app/src/main')
 this.generateIOSIcons(args.file, `ios/${flags.appName}/Images.xcassets/AppIcon.appiconset`)
 ```
 
-**In splash.ts (has prefix):**
+**In splash.ts (updated to remove prefix):**
 ```typescript
+// Before:
 this.generateAndroidSplashscreens(args.file, './android/app/src/main/res')
 this.generateIOSSplashscreens(args.file, `./ios/${flags.appName}/Images.xcassets/Splashscreen.imageset`)
+
+// After:
+this.generateAndroidSplashscreens(args.file, 'android/app/src/main/res')
+this.generateIOSSplashscreens(args.file, `ios/${flags.appName}/Images.xcassets/Splashscreen.imageset`)
 ```
 
-**Recommendation:** Remove `./` prefix from splash.ts paths for consistency with icons.ts and standard Node.js path handling.
+### 5. Improve `extractAppName` Error Handling ✅ COMPLETED
 
-### 5. Improve `extractAppName` Error Handling (Priority: Low)
+**Status:** ✅ Completed in PR #XX
 
 **File:** `src/utils/app.utils.ts`
 
-Current implementation silently returns `null` for all error cases. Consider more specific error handling to help users debug issues.
+Improved implementation with explicit type annotation and better validation.
 
 **Current:**
 ```typescript
@@ -180,7 +223,7 @@ export function extractAppName() {
 }
 ```
 
-**Suggested improvement:**
+**Implemented improvement:**
 ```typescript
 export function extractAppName(): string | null {
   try {
@@ -202,7 +245,9 @@ export function extractAppName(): string | null {
 
 ## Refactoring Suggestions (Lower Priority)
 
-### 1. Extract Base Command Class (Priority: Medium)
+### 1. Extract Base Command Class ⏳ PENDING
+
+**Status:** ⏳ Not yet implemented (Priority: Medium)
 
 **Issue:** Both `Icons` and `Splash` commands duplicate the same `logVerbose()` implementation and `_isVerbose` property.
 
@@ -232,7 +277,9 @@ export abstract class BaseCommand extends Command {
 
 **Then update commands to extend `BaseCommand` instead of `Command`.**
 
-### 2. Add Type Definitions for Icon Sizes (Priority: Low)
+### 2. Add Type Definitions for Icon Sizes ⏳ PENDING
+
+**Status:** ⏳ Not yet implemented (Priority: Low)
 
 **File:** `src/types.ts`
 
@@ -258,7 +305,9 @@ export const ICON_SIZES_ANDROID: Array<IconSizeAndroid> = [...]
 export const ICON_SIZES_IOS: Array<IconSizeIOS> = [...]
 ```
 
-### 3. Improve Error Collection (Priority: Medium)
+### 3. Improve Error Collection ⏳ PENDING
+
+**Status:** ⏳ Not yet implemented (Priority: Medium)
 
 **Issue:** In `icons.ts` and `splash.ts`, errors during image generation are logged but execution continues silently. Users may not realize some icons failed to generate.
 
@@ -285,20 +334,22 @@ if (this.errors.length > 0) {
 
 | File | Test File | Status |
 |------|-----------|--------|
-| `src/commands/icons.ts` | `test/commands/icons.test.ts` | Needs verbose tests, fix typo, standardize paths |
-| `src/commands/splash.ts` | `test/commands/splash.test.ts` | Needs verbose tests, fix typo, standardize paths |
-| `src/commands/dotenv.ts` | `test/commands/dotenv.test.ts` | Needs verbose flag + tests |
-| `src/utils/app.utils.ts` | ❌ Missing | Create new test file, improve error handling |
-| `src/utils/file-utils.ts` | ❌ Missing | Consider adding tests |
-| `src/utils/color.utils.ts` | ❌ Missing | Consider adding tests |
-| `src/constants.ts` | N/A | Add license header, add types |
-| `src/types.ts` | N/A | Add icon size interfaces |
+| `src/commands/icons.ts` | `test/commands/icons.test.ts` | ✅ Verbose tests added, typo fixed |
+| `src/commands/splash.ts` | `test/commands/splash.test.ts` | ✅ Verbose tests added, typo fixed, paths standardized |
+| `src/commands/dotenv.ts` | `test/commands/dotenv.test.ts` | ✅ Verbose flag + tests added |
+| `src/utils/app.utils.ts` | `test/app.utils.test.ts` | ✅ Test file created, error handling improved |
+| `src/utils/file-utils.ts` | ❌ Missing | ⏳ Consider adding tests |
+| `src/utils/color.utils.ts` | ❌ Missing | ⏳ Consider adding tests |
+| `src/constants.ts` | N/A | ✅ License header added |
+| `src/types.ts` | N/A | ⏳ Icon size interfaces pending |
 
 ---
 
 ## Priority Summary
 
-| Priority | Count | Items |
-|----------|-------|-------|
-| 🔶 Medium | 5 | Base command extraction, error collection, verbose tests, dotenv verbose flag, malformed input tests |
-| 🔷 Low | 6 | Typo fix, license header, path prefixes, extractAppName improvement, icon size types, utility tests |
+| Priority | Completed | Pending | Items |
+|----------|-----------|---------|-------|
+| 🔶 Medium | 2 | 3 | ✅ Verbose tests, dotenv verbose flag ⏳ Base command extraction, error collection, malformed input tests |
+| 🔷 Low | 4 | 2 | ✅ Typo fix, license header, path prefixes, extractAppName improvement ⏳ Icon size types, utility tests |
+
+**Overall Progress:** 6/11 items completed (55%)

src/commands/dotenv.ts

@@ -22,10 +22,18 @@ export default class Dotenv extends Command {
   ]
   static override flags = {
     help: Flags.help({ char: 'h' }),
+    verbose: Flags.boolean({
+      char: 'v',
+      default: false,
+      description: 'Print more detailed log messages.',
+    }),
   }
+  private _isVerbose: boolean = false
 
   public async run(): Promise<void> {
-    const { args } = await this.parse(Dotenv)
+    const { args, flags } = await this.parse(Dotenv)
+
+    this._isVerbose = flags.verbose
 
     const sourceEnvFilePath = `./.env.${args.environmentName}`
     const outputEnvFile = './.env'
@@ -35,24 +43,31 @@ export default class Dotenv extends Command {
       this.error(`Source file ${cyan(sourceEnvFilePath)} not found! ${red('ABORTING')}`)
     }
 
+    this.logVerbose(`${yellow('≈')} Source environment file: ${cyan(sourceEnvFilePath)}`)
     this.log(`${yellow('≈')} Generating .env from ${cyan(sourceEnvFilePath)} file...`)
 
     // Remove existing .env file
-    this.log(`${yellow('≈')} Removing existing .env file (if any)...`)
+    this.logVerbose(`${yellow('≈')} Removing existing .env file (if any)...`)
     try {
       await unlink(outputEnvFile)
-      this.log(`${green('✔')} Removed existing .env file.`)
+      this.logVerbose(`${green('✔')} Removed existing .env file.`)
     } catch {
-      this.log(`${red('✘')} No existing .env file to remove.`)
+      this.logVerbose(`${red('✘')} No existing .env file to remove.`)
     }
 
     // Copy new .env file
-    this.log(`${yellow('≈')} Generating new .env file...`)
+    this.logVerbose(`${yellow('≈')} Generating new .env file...`)
     try {
       await copyFile(sourceEnvFilePath, outputEnvFile)
       this.log(`${green('✔')} Generated new .env file.`)
     } catch (error) {
       this.error(error as Error)
     }
   }
+
+  private logVerbose(message?: string, ...args: unknown[]) {
+    if (this._isVerbose) {
+      this.log(message, ...args)
+    }
+  }
 }

src/commands/icons.ts

@@ -62,7 +62,7 @@ The template icon file should be at least 1024x1024px.
     }
 
     if (!flags.appName) {
-      this.error(`${red('✘')} Failed to retrive ${cyan('appName')} value. Please specify it with the ${green('appName')} flag or check that ${cyan('app.json')} file exists. ${red('ABORTING')}`)
+      this.error(`${red('✘')} Failed to retrieve ${cyan('appName')} value. Please specify it with the ${green('appName')} flag or check that ${cyan('app.json')} file exists. ${red('ABORTING')}`)
     }
 
     this.log(yellow('≈'), `Generating icons for '${cyan(flags.appName)}' app...`)

src/commands/splash.ts

@@ -61,15 +61,15 @@ The template splashscreen file should be at least 1242x2208px.
     }
 
     if (!flags.appName) {
-      this.error(`${red('✘')} Failed to retrive ${cyan('appName')} value. Please specify it with the ${green('appName')} flag or check that ${cyan('app.json')} file exists. ${red('ABORTING')}`)
+      this.error(`${red('✘')} Failed to retrieve ${cyan('appName')} value. Please specify it with the ${green('appName')} flag or check that ${cyan('app.json')} file exists. ${red('ABORTING')}`)
     }
 
     this.log(yellow('≈'), `Generating splashscreens for '${cyan(flags.appName)}' app...`)
 
     // Run both iOS and Android tasks in parallel
     await Promise.all([
-      this.generateAndroidSplashscreens(args.file, './android/app/src/main/res'),
-      this.generateIOSSplashscreens(args.file, `./ios/${flags.appName}/Images.xcassets/Splashscreen.imageset`),
+      this.generateAndroidSplashscreens(args.file, 'android/app/src/main/res'),
+      this.generateIOSSplashscreens(args.file, `ios/${flags.appName}/Images.xcassets/Splashscreen.imageset`),
     ])
 
     this.log(green('✔'), `Generated splashscreens for '${cyan(flags.appName)}' app.`)

src/constants.ts

@@ -1,3 +1,11 @@
+/*
+ * Copyright (c) 2025 ForWarD Software (https://forwardsoftware.solutions/)
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
 import type { SplashscreenSize } from "./types.js"
 
 /**