Skip to content

Auto-add file extension when missing #954

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

Closed
yamadashy wants to merge 1 commit into from
Closed

Auto-add file extension when missing #954

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: 0 additions & 8 deletions 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.

18 changes: 18 additions & 0 deletions src/config/configLoad.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
type RepomixConfigCli,
type RepomixConfigFile,
type RepomixConfigMerged,
type RepomixOutputStyle,
repomixConfigFileSchema,
repomixConfigMergedSchema,
} from './configSchema.js';
Expand Down Expand Up @@ -167,6 +168,14 @@ const loadAndValidateConfig = async (
}
};

// Mapping of output styles to their file extensions
const styleToExtensionMap: Record<RepomixOutputStyle, string> = {
Copy link
Copy Markdown
Contributor

@claude claude bot Nov 16, 2025

Choose a reason for hiding this comment

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

Code Quality: Consider Making Map More Maintainable

The styleToExtensionMap duplicates information from defaultFilePathMap. Consider deriving one from the other to maintain a single source of truth.

Suggested approach:

// Derive extensions from defaultFilePathMap
const styleToExtensionMap: Record<RepomixOutputStyle, string> = Object.fromEntries(
  Object.entries(defaultFilePathMap).map(([style, filePath]) => [
    style,
    path.extname(filePath)
  ])
) as Record<RepomixOutputStyle, string>;

This ensures consistency and reduces maintenance burden when adding new output styles.

xml: '.xml',
markdown: '.md',
plain: '.txt',
json: '.json',
} as const;
Comment on lines +172 to +177
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Nov 16, 2025

Choose a reason for hiding this comment

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

medium

While the current implementation is correct, the type assertion as const combined with an explicit type annotation Record<RepomixOutputStyle, string> is a bit redundant. You can simplify this by letting TypeScript infer the type from the as const assertion. This makes the code more concise and still ensures type safety and immutability.

Suggested change
const styleToExtensionMap: Record<RepomixOutputStyle, string> = {
xml: '.xml',
markdown: '.md',
plain: '.txt',
json: '.json',
} as const;
const styleToExtensionMap = {
xml: '.xml',
markdown: '.md',
plain: '.txt',
json: '.json',
} as const;


export const mergeConfigs = (
cwd: string,
fileConfig: RepomixConfigFile,
Expand Down Expand Up @@ -205,6 +214,15 @@ export const mergeConfigs = (
mergedOutput.filePath = desiredPath;
logger.trace('Adjusted output file path to match style:', mergedOutput.filePath);
}
} else {
// If filePath is explicitly set, check if it has an extension
const currentExtension = path.extname(mergedOutput.filePath);
Copy link
Copy Markdown
Contributor

@claude claude bot Nov 16, 2025

Choose a reason for hiding this comment

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

Potential Issue: Edge Case with Dotfiles

The current logic uses path.extname() which returns an empty string for files like .gitignore or .env. This means if a user specifies an output path like .myoutput, the system will add an extension making it .myoutput.xml.

Example:

// Current behavior:
path.extname('.myoutput') // returns ''
// Result: '.myoutput.xml'

// Expected behavior: probably preserve '.myoutput' as-is?

Suggested fix:

const currentExtension = path.extname(mergedOutput.filePath);
const basename = path.basename(mergedOutput.filePath);
const isDotfile = basename.startsWith('.') && !basename.includes('.', 1);

if (!currentExtension && !isDotfile) {
  const extensionToAdd = styleToExtensionMap[style];
  mergedOutput.filePath = `${mergedOutput.filePath}${extensionToAdd}`;
  logger.trace('Added file extension to output path:', mergedOutput.filePath);
}

This ensures dotfiles are preserved unchanged.

if (!currentExtension) {
// No extension found, add the appropriate extension based on style
const extensionToAdd = styleToExtensionMap[style];
mergedOutput.filePath = `${mergedOutput.filePath}${extensionToAdd}`;
logger.trace('Added file extension to output path:', mergedOutput.filePath);
}
Comment on lines +219 to +225
Copy link

Copilot AI Nov 16, 2025

Choose a reason for hiding this comment

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

Files starting with a dot (like .gitignore or .env) will be treated as having an extension by path.extname(). For example, path.extname('.config') returns '.config', meaning the extension will not be added when it should be. Consider checking if the filename starts with a dot and has no other extension, or use a more robust check like path.basename(mergedOutput.filePath).indexOf('.') === 0.

Copilot uses AI. Check for mistakes.
}

return mergedOutput;
Expand Down
60 changes: 60 additions & 0 deletions tests/config/configLoad.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,5 +329,65 @@ describe('configLoad', () => {
expect(merged.output.filePath).toBe('repomix-output.txt');
expect(merged.output.style).toBe('plain');
});

test('should add extension when CLI filePath has no extension (default style)', () => {
const merged = mergeConfigs(process.cwd(), {}, { output: { filePath: 'myoutput' } });
expect(merged.output.filePath).toBe('myoutput.xml');
expect(merged.output.style).toBe('xml');
});

test('should add extension when CLI filePath has no extension (markdown style)', () => {
const merged = mergeConfigs(process.cwd(), {}, { output: { filePath: 'myoutput', style: 'markdown' } });
expect(merged.output.filePath).toBe('myoutput.md');
expect(merged.output.style).toBe('markdown');
});

test('should add extension when CLI filePath has no extension (plain style)', () => {
const merged = mergeConfigs(process.cwd(), {}, { output: { filePath: 'myoutput', style: 'plain' } });
expect(merged.output.filePath).toBe('myoutput.txt');
expect(merged.output.style).toBe('plain');
});

test('should add extension when CLI filePath has no extension (json style)', () => {
const merged = mergeConfigs(process.cwd(), {}, { output: { filePath: 'myoutput', style: 'json' } });
expect(merged.output.filePath).toBe('myoutput.json');
expect(merged.output.style).toBe('json');
});
Comment on lines +333 to +355
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Nov 16, 2025

Choose a reason for hiding this comment

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

medium

These tests are great for ensuring correctness! To improve maintainability and reduce code duplication, you could parameterize these related test cases using test.each. This will make it easier to add or modify tests for different output styles in the future.

    test.each([
      { description: 'default style', config: { output: { filePath: 'myoutput' } }, expectedStyle: 'xml', expectedExt: '.xml' },
      { description: 'markdown style', config: { output: { filePath: 'myoutput', style: 'markdown' } }, expectedStyle: 'markdown', expectedExt: '.md' },
      { description: 'plain style', config: { output: { filePath: 'myoutput', style: 'plain' } }, expectedStyle: 'plain', expectedExt: '.txt' },
      { description: 'json style', config: { output: { filePath: 'myoutput', style: 'json' } }, expectedStyle: 'json', expectedExt: '.json' },
    ])('should add extension when CLI filePath has no extension ($description)', ({ config, expectedStyle, expectedExt }) => {
      const merged = mergeConfigs(process.cwd(), {}, config);
      expect(merged.output.filePath).toBe(`myoutput${expectedExt}`);
      expect(merged.output.style).toBe(expectedStyle);
    });


test('should keep extension when CLI filePath already has extension', () => {
const merged = mergeConfigs(process.cwd(), {}, { output: { filePath: 'myoutput.txt', style: 'markdown' } });
expect(merged.output.filePath).toBe('myoutput.txt');
expect(merged.output.style).toBe('markdown');
});

test('should add extension when file config filePath has no extension', () => {
const merged = mergeConfigs(process.cwd(), { output: { filePath: 'myoutput', style: 'markdown' } }, {});
expect(merged.output.filePath).toBe('myoutput.md');
expect(merged.output.style).toBe('markdown');
});

test('should keep extension when file config filePath already has extension', () => {
const merged = mergeConfigs(process.cwd(), { output: { filePath: 'myoutput.custom', style: 'markdown' } }, {});
expect(merged.output.filePath).toBe('myoutput.custom');
expect(merged.output.style).toBe('markdown');
});

test('should add extension for paths with directories when no extension', () => {
const merged = mergeConfigs(process.cwd(), {}, { output: { filePath: 'output/myfile', style: 'json' } });
expect(merged.output.filePath).toBe('output/myfile.json');
expect(merged.output.style).toBe('json');
});

test('should keep extension for paths with directories when extension exists', () => {
const merged = mergeConfigs(process.cwd(), {}, { output: { filePath: 'output/myfile.txt', style: 'json' } });
expect(merged.output.filePath).toBe('output/myfile.txt');
expect(merged.output.style).toBe('json');
});

test('should add extension when style is changed via CLI but filePath has no extension', () => {
const merged = mergeConfigs(process.cwd(), { output: { filePath: 'myfile' } }, { output: { style: 'markdown' } });
expect(merged.output.filePath).toBe('myfile.md');
expect(merged.output.style).toBe('markdown');
});
});
});
Loading