Skip to content

fix: Map short options in generate command to prevent silent exit #183

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nayrbryanGaming wants to merge 1 commit into
base: master
Choose a base branch
from
+30 −1
Open

fix: Map short options in generate command to prevent silent exit #183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions src/commands/generate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ export default class Generate extends Command {
'Slash Command Extension',
'Settings Extension',
];

if (option) {
const index = ['a', 'b', 'c'].indexOf(option);
if (index !== -1) {
option = categories[index];
}
Comment on lines +36 to +41
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shorthand mapping uses two parallel arrays (['a','b','c'] and categories) and relies on them staying in sync by index. To avoid future mismatches when categories change, prefer a single explicit mapping (e.g., a record keyed by the short option) or switch directly on the short values.

Suggested change
if (option) {
const index = ['a', 'b', 'c'].indexOf(option);
if (index !== -1) {
option = categories[index];
}
const shorthandMap: { [key: string]: string } = {
a: categories[0],
b: categories[1],
c: categories[2],
};
if (option && shorthandMap[option]) {
option = shorthandMap[option];

Copilot uses AI. Check for mistakes.
}

if (!option) {
inquirer.registerPrompt('checkbox-plus', require('inquirer-checkbox-plus-prompt'));
const result = await inquirer.prompt([{
Expand Down
15 changes: 15 additions & 0 deletions test/commands/generate.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { test } from '@oclif/test';

describe('generate', () => {
test
.stdout()
.command(['generate', '-o', 'a'])
.exit(2)
.it('runs generate with option "a" and correctly attempts to find app.json');

test
.stdout()
.command(['generate', '-o', 'x'])
.exit(2)
.it('fails when passing an invalid option not in a,b,c');
Comment on lines +2 to +14
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new test cases exit during FolderDetails.readInfoFile() when app.json is missing, so they don't exercise the new shorthand mapping logic (the code that maps 'a'/'b'/'c' to category strings runs after readInfoFile). Consider adding a fixture (e.g., temp cwd with a minimal app.json + classFile) so the command reaches the switch block, and assert on an observable effect (generated file path, invoked prompt, or expected error message for invalid option).

Suggested change
describe('generate', () => {
test
.stdout()
.command(['generate', '-o', 'a'])
.exit(2)
.it('runs generate with option "a" and correctly attempts to find app.json');
test
.stdout()
.command(['generate', '-o', 'x'])
.exit(2)
.it('fails when passing an invalid option not in a,b,c');
import * as fs from 'fs';
import * as path from 'path';
import * as os from 'os';
describe('generate', () => {
let tempDir: string;
beforeEach(() => {
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'generate-test-'));
// Minimal app.json so FolderDetails.readInfoFile() succeeds
const appJsonPath = path.join(tempDir, 'app.json');
fs.writeFileSync(appJsonPath, JSON.stringify({ name: 'test-app' }), 'utf8');
// Dummy class file if the command expects one to exist
const classFilePath = path.join(tempDir, 'classFile.cls');
fs.writeFileSync(classFilePath, '// dummy class file\n', 'utf8');
});
afterEach(() => {
if (tempDir && fs.existsSync(tempDir)) {
fs.rmSync(tempDir, { recursive: true, force: true });
}
});
test
.stdout()
.cwd(() => tempDir)
.command(['generate', '-o', 'a'])
.it('runs generate with option "a" using a valid app.json fixture');
test
.stderr()
.cwd(() => tempDir)
.command(['generate', '-o', 'x'])
.exit(2)
.it('fails when passing an invalid option not in a,b,c', ctx => {
if (!ctx.stderr || ctx.stderr.length === 0) {
throw new Error('Expected error output for invalid option "x".');
}
});

Copilot uses AI. Check for mistakes.
Comment on lines +2 to +14
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test only checks for exit code and doesn't assert on stdout/stderr content, so it may pass even if the command silently exits again. Adding an assertion on the expected error output (invalid flag value) and/or a side effect for the valid shorthand case would make this regression test meaningful.

Suggested change
describe('generate', () => {
test
.stdout()
.command(['generate', '-o', 'a'])
.exit(2)
.it('runs generate with option "a" and correctly attempts to find app.json');
test
.stdout()
.command(['generate', '-o', 'x'])
.exit(2)
.it('fails when passing an invalid option not in a,b,c');
import { expect } from 'chai';
describe('generate', () => {
test
.stdout()
.command(['generate', '-o', 'a'])
.exit(2)
.it('runs generate with option "a" and correctly attempts to find app.json', ctx => {
expect(ctx.stdout).to.not.equal('');
});
test
.stdout()
.stderr()
.command(['generate', '-o', 'x'])
.exit(2)
.it('fails when passing an invalid option not in a,b,c', ctx => {
expect(ctx.stderr).to.not.equal('');
});

Copilot uses AI. Check for mistakes.
});
Loading