Skip to content

Feat/34 safe ouput dir overwrite#39

Merged
pavelsvagr merged 1 commit into
mainfrom
feat/34-safe-ouput-dir-overwrite
Aug 8, 2025
Merged

Feat/34 safe ouput dir overwrite#39
pavelsvagr merged 1 commit into
mainfrom
feat/34-safe-ouput-dir-overwrite

Conversation

@mashabek

Copy link
Copy Markdown
Collaborator

Stop if destination directory exists. Add force overwrite destination directory flag.

Fixes #34

@pavelsvagr pavelsvagr force-pushed the feat/34-safe-ouput-dir-overwrite branch 3 times, most recently from 7e9c637 to e6b897e Compare July 28, 2025 10:58
@pavelsvagr pavelsvagr requested a review from hofp37 July 28, 2025 10:59
@pavelsvagr pavelsvagr self-assigned this Jul 28, 2025
@pavelsvagr pavelsvagr force-pushed the feat/34-safe-ouput-dir-overwrite branch from e6b897e to dc493e8 Compare July 28, 2025 11:41
Comment thread src/Bootstrap.ts

if (toolbelt.isDirectoryNonEmpty(destination)) {
if (!parsedArgs.force) {
logger.info(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Optional: Single log could be used

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Added to single log

Comment thread src/Toolbelt.ts Outdated
return path.normalize(str) as Path
}

public isDirectoryNonEmpty(dirpath: string): boolean {

@hofp37 hofp37 Jul 31, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Optional: Having isDirectoryEmpty instead would be more clearer - maybe just my POV

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Refactored, I like the idea. 👍

Comment thread src/Toolbelt.ts Outdated
if (!rootPath.includes(dirpath)) {
if (fs.existsSync(dirpath) && option?.overwrite) {
fs.rmSync(dirpath, { recursive: true })
if (fs.existsSync(dirpath)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Something like this would seem easier to read to me. What do you think?

if (fs.existsSync(dirpath) && option?.overwrite) {
    fs.rmSync(dirpath, { recursive: true })
}
fs.mkdirSync(dirpath, { recursive: true })

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Done, thanks!

@pavelsvagr pavelsvagr force-pushed the feat/34-safe-ouput-dir-overwrite branch from dc493e8 to b522871 Compare August 4, 2025 15:42
@pavelsvagr pavelsvagr merged commit 1480e80 into main Aug 8, 2025
4 checks passed
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.

✨ Don't overwrite target directory by default

3 participants