Skip to content

feat(toFile): the situation where compatible paths cannot exist#84

Open
fanxinqi wants to merge 1 commit intohuan:mainfrom
fanxinqi:main
Open

feat(toFile): the situation where compatible paths cannot exist#84
fanxinqi wants to merge 1 commit intohuan:mainfrom
fanxinqi:main

Conversation

@fanxinqi
Copy link

@fanxinqi fanxinqi commented Mar 6, 2024

If the path cannot exist, create the path
If there is a path, follow the original logic normally

If the path cannot exist, create the path
If there is a path, follow the original logic normally
Copy link
Owner

@huan huan left a comment

Choose a reason for hiding this comment

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

Please modify code as requested.

And,

  1. please add a param comment (jsdoc) to the overwrite parameter to explain the new feature.
  2. please add an unit test to test this new feature.

// Check if file exists
let exist = false;
try {
FS.accessSync(fullFilePath);
Copy link
Owner

Choose a reason for hiding this comment

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

use async instead of sync, with await, please

// Create directory if it doesn't exist
const directory = PATH.dirname(fullFilePath);
if (!FS.existsSync(directory)) {
FS.mkdirSync(directory, { recursive: true });
Copy link
Owner

Choose a reason for hiding this comment

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

only do the mkdir if the overwrite is true

@huan huan requested a review from Copilot April 13, 2025 06:46
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.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/file-box.ts:903

  • [nitpick] Consider renaming the variable 'exist' to 'exists' to better reflect that it represents a boolean value, which would improve clarity.
let exist = false;

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