Skip to content

Conversation

@hsb-tonmoy
Copy link

Fixes Issue #1

@betomoedano
Copy link
Owner

Hi @hsb-tonmoy,

Thanks for the PR, I released a fix for that this morning. Please let me know if that works for you.

Otherwise, I'm happy to revisit this PR.

Thanks

@betomoedano betomoedano requested a review from Copilot June 24, 2025 12:59
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Fixes Windows path resolution by converting import.meta.url to a file system path for the execute command’s dir parameter.

  • Suppress punycode deprecation warnings with updated quoting style
  • Import and use fileURLToPath and dirname from Node core modules
  • Compute __dirname and pass it to execute instead of import.meta.url
Comments suppressed due to low confidence (3)

src/index.ts:19

  • [nitpick] The name __dirname may be confused with the CommonJS global; consider renaming it to something like currentDir or projectDir to clarify its purpose.
const __dirname = dirname(fileURLToPath(import.meta.url));

src/index.ts:19

  • Add a brief comment explaining why fileURLToPath and dirname are used to compute the directory path for cross-platform compatibility.
const __dirname = dirname(fileURLToPath(import.meta.url));

src/index.ts:21

  • Add tests to verify that Windows-style paths are correctly resolved and passed to execute, ensuring the fix addresses the original issue.
await execute({ dir: __dirname });

process.removeAllListeners('warning');
process.on('warning', (warning) => {
if (warning.name === 'DeprecationWarning' && warning.message.includes('punycode')) {
process.removeAllListeners("warning");
Copy link

Copilot AI Jun 24, 2025

Choose a reason for hiding this comment

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

Removing all listeners for the 'warning' event can inadvertently disable other handlers; consider filtering only punycode deprecation warnings or document this side effect.

Suggested change
process.removeAllListeners("warning");

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants