Skip to content

Conversation

antongolub
Copy link
Collaborator

  • Tests pass
  • Appropriate changes to README are included in PR

@antongolub antongolub added the refactoring Internal code improvements label Aug 11, 2025
@antonmedv
Copy link
Collaborator

Why this change?

@antongolub
Copy link
Collaborator Author

Aligns overload implementations across the code: replaces args mutations with a recursive call.

@antongolub antongolub requested a review from Copilot August 11, 2025 10:27
Copy link

@Copilot 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

This PR refactors the cd() function to avoid mutating the input argument by using recursion instead of reassignment when handling ProcessOutput instances.

  • Replaces argument mutation with recursive call pattern
  • Adds explicit void return type annotation to the TypeScript version

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/core.ts Refactors cd() to use recursion instead of mutating the dir parameter
build/core.cjs Compiled JavaScript output reflecting the same refactoring changes

Comment on lines +1030 to 1033
if (dir instanceof ProcessOutput) return cd(dir.toString().trim())

$.log({ kind: 'cd', dir, verbose: !$.quiet && $.verbose })
process.chdir(dir)
Copy link
Preview

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

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

The recursive call creates unnecessary function call overhead. Consider using a local variable assignment instead: const dirStr = dir instanceof ProcessOutput ? dir.toString().trim() : dir; then proceed with the rest of the function using dirStr.

Suggested change
if (dir instanceof ProcessOutput) return cd(dir.toString().trim())
$.log({ kind: 'cd', dir, verbose: !$.quiet && $.verbose })
process.chdir(dir)
const dirStr = dir instanceof ProcessOutput ? dir.toString().trim() : dir;
$.log({ kind: 'cd', dir: dirStr, verbose: !$.quiet && $.verbose })
process.chdir(dirStr)

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
refactoring Internal code improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants