Skip to content
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.

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

Enhance performance and user experience with improved progress reporting #65

Merged
merged 2 commits into from
Aug 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion src/cli/actions/defaultActionRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ export const runDefaultAction = async (
let packResult: PackResult;

try {
packResult = await pack(targetPath, config);
packResult = await pack(targetPath, config, (message) => {
spinner.update(message);
});
} catch (error) {
spinner.fail('Error during packing');
throw error;
Expand Down
2 changes: 2 additions & 0 deletions src/cli/actions/remoteActionRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ export const runRemoteAction = async (repoUrl: string, options: CliOptions): Pro
spinner.start();
await cloneRepository(formattedUrl, tempDir);
spinner.succeed('Repository cloned successfully!');
logger.log('');

const result = await runDefaultAction(tempDir, tempDir, options);
await copyOutputToCurrentDirectory(tempDir, process.cwd(), result.config.output.filePath);
} finally {
Expand Down
10 changes: 8 additions & 2 deletions src/cli/cliSpinner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,19 @@ class Spinner {
}

start(): void {
const frames = this.spinner.frames;
const framesLength = frames.length;
this.interval = setInterval(() => {
const frame = this.spinner.frames[this.currentFrame];
this.currentFrame++;
const frame = frames[this.currentFrame % framesLength];
logUpdate(`${pc.cyan(frame)} ${this.message}`);
this.currentFrame = (this.currentFrame + 1) % this.spinner.frames.length;
}, this.spinner.interval);
}

update(message: string): void {
this.message = message;
}

stop(finalMessage: string): void {
if (this.interval) {
clearInterval(this.interval);
Expand Down
11 changes: 11 additions & 0 deletions src/config/defaultIgnore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,4 +118,15 @@ export const defaultIgnoreList = [

// repopack output
'repopack-output.txt',

// Essential Python-related entries
'**/__pycache__/**',
'**/*.py[cod]',
'venv/**',
'.venv/**',
'.pytest_cache/**',
'.mypy_cache/**',
'.ipynb_checkpoints/**',
'Pipfile.lock',
'poetry.lock',
];
23 changes: 20 additions & 3 deletions src/core/packager.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import * as fs from 'node:fs/promises';
import fs from 'node:fs/promises';
import path from 'node:path';
import pMap from 'p-map';
import type { RepopackConfigMerged } from '../config/configTypes.js';
import { logger } from '../shared/logger.js';
import { getProcessConcurrency } from '../shared/processConcurrency.js';
import { sleep } from '../shared/sleep.js';
import type { RepopackProgressCallback } from '../shared/types.js';
import { collectFiles as defaultCollectFiles } from './file/fileCollector.js';
import { processFiles as defaultProcessFiles } from './file/fileProcessor.js';
import { searchFiles as defaultSearchFiles } from './file/fileSearcher.js';
Expand Down Expand Up @@ -34,6 +36,7 @@ export interface PackResult {
export const pack = async (
rootDir: string,
config: RepopackConfigMerged,
progressCallback: RepopackProgressCallback = () => {},
deps: PackDependencies = {
searchFiles: defaultSearchFiles,
collectFiles: defaultCollectFiles,
Expand All @@ -43,25 +46,32 @@ export const pack = async (
},
): Promise<PackResult> => {
// Get all file paths considering the config
progressCallback('Searching for files...');
const filePaths = await deps.searchFiles(rootDir, config);

// Collect raw files
progressCallback('Collecting files...');
const rawFiles = await deps.collectFiles(filePaths, rootDir);

// Perform security check and filter out suspicious files
const suspiciousFilesResults = await deps.runSecurityCheck(rawFiles);
progressCallback('Running security check...');
const suspiciousFilesResults = await deps.runSecurityCheck(rawFiles, progressCallback);
const safeRawFiles = rawFiles.filter(
(rawFile) => !suspiciousFilesResults.some((result) => result.filePath === rawFile.path),
);
const safeFilePaths = safeRawFiles.map((file) => file.path);
logger.trace('Safe files count:', safeRawFiles.length);

// Process files (remove comments, etc.)
progressCallback('Processing files...');
const processedFiles = await deps.processFiles(safeRawFiles, config);

// Generate output
progressCallback('Generating output...');
const output = await deps.generateOutput(config, processedFiles, safeFilePaths);

// Write output to file. path is relative to the cwd
progressCallback('Writing output file...');
const outputPath = path.resolve(config.cwd, config.output.filePath);
logger.trace(`Writing output to: ${outputPath}`);
await fs.writeFile(outputPath, output);
Expand All @@ -70,11 +80,18 @@ export const pack = async (
const tokenCounter = new TokenCounter();

// Metrics
progressCallback('Calculating metrics...');
const fileMetrics = await pMap(
processedFiles,
async (file) => {
async (file, index) => {
const charCount = file.content.length;
const tokenCount = tokenCounter.countTokens(file.content);

progressCallback(`Calculating metrics... (${index + 1}/${processedFiles.length})`);

// Sleep for a short time to prevent blocking the event loop
await sleep(1);

return { path: file.path, charCount, tokenCount };
},
{
Expand Down
16 changes: 14 additions & 2 deletions src/core/security/securityCheckRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,38 @@ import type { SecretLintCoreConfig, SecretLintCoreResult } from '@secretlint/typ
import pMap from 'p-map';
import { logger } from '../../shared/logger.js';
import { getProcessConcurrency } from '../../shared/processConcurrency.js';
import { sleep } from '../../shared/sleep.js';
import type { RepopackProgressCallback } from '../../shared/types.js';
import type { RawFile } from '../file/fileTypes.js';

export interface SuspiciousFileResult {
filePath: string;
messages: string[];
}

export const runSecurityCheck = async (rawFiles: RawFile[]): Promise<SuspiciousFileResult[]> => {
export const runSecurityCheck = async (
rawFiles: RawFile[],
progressCallback: RepopackProgressCallback = () => {},
): Promise<SuspiciousFileResult[]> => {
const secretLintConfig = createSecretLintConfig();

const results = await pMap(
rawFiles,
async (rawFile) => {
async (rawFile, index) => {
const secretLintResult = await runSecretLint(rawFile.path, rawFile.content, secretLintConfig);

if (secretLintResult.messages.length > 0) {
return {
filePath: rawFile.path,
messages: secretLintResult.messages.map((message) => message.message),
};
}

progressCallback(`Running security check... (${index + 1}/${rawFiles.length})`);

// Sleep for a short time to prevent blocking the event loop
await sleep(1);

return null;
},
{
Expand Down
1 change: 1 addition & 0 deletions src/shared/sleep.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const sleep = (ms: number): Promise<void> => new Promise((resolve) => setTimeout(resolve, ms));
1 change: 1 addition & 0 deletions src/shared/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export type RepopackProgressCallback = (message: string) => void;
4 changes: 2 additions & 2 deletions tests/core/packager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('packager', () => {
test('pack should process files and generate output', async () => {
const mockConfig = createMockConfig();

const result = await pack('root', mockConfig, mockDeps);
const result = await pack('root', mockConfig, () => {}, mockDeps);

const file2Path = path.join('dir1', 'file2.txt');

Expand Down Expand Up @@ -79,7 +79,7 @@ describe('packager', () => {
},
]);

const result = await pack('root', mockConfig, mockDeps);
const result = await pack('root', mockConfig, () => {}, mockDeps);

expect(mockDeps.searchFiles).toHaveBeenCalledWith('root', mockConfig);
expect(mockDeps.processFiles).toHaveBeenCalledWith(
Expand Down
Loading