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

perf: handle the case if the file is not present. #812

Merged
merged 8 commits into from
Oct 9, 2023
13 changes: 9 additions & 4 deletions src/commands/new/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,16 @@ export default class NewFile extends Command {
console.log(`File ${fileNameToWriteToDisk} already exists. Ignoring...`);
return;
}
} catch (e) {
// File does not exist. Proceed creating it...
} catch (e:any ) {

if (e.code === 'EACCES') {
console.error('Permission denied to read the file. You do not have the necessary permissions.');
} else {
await writeFile(fileNameToWriteToDisk, asyncApiFile, { encoding: 'utf8' });
Copy link
Member

Choose a reason for hiding this comment

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

I would leave the "creation of the file" outside the catch, so it's not be treated as an error like I explain here.

Choose a reason for hiding this comment

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

ok @peter-rr will do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@peter-rr should i create file even though there is the permission error? means even though we dont have permission to read the file we just created it. it will look like this.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

till then i have ran the lint command please consider merging it.

Copy link
Member

Choose a reason for hiding this comment

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

@peter-rr should i create file even though there is the permission error? means even though we dont have permission to read the file we just created it. it will look like this.

Nope, we should not create the file if a permission error occurs. In that case we should throw the error so that we exit the execution and the file is not created then:

} catch (e) {
      if (e.code === 'EACCES') {
        throw e;
      }
}
await writeFile(fileNameToWriteToDisk, asyncApiFile, { encoding: 'utf8' });
console.log(`Created file ${fileNameToWriteToDisk}...`);

Another alternative we have is using this.error instead, since OCLIF makes use of it as console.error + throw e:

} catch (e) {
      if (e.code === 'EACCES') {
        this.error('Permission denied to read the file. You do not have the necessary permissions.');
      }
}
await writeFile(fileNameToWriteToDisk, asyncApiFile, { encoding: 'utf8' });
console.log(`Created file ${fileNameToWriteToDisk}...`);

Copy link
Contributor Author

@ayushnau ayushnau Sep 28, 2023

Choose a reason for hiding this comment

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

ok will do that. @peter-rr

console.log(`Created file ${fileNameToWriteToDisk}...`);

}
}

await writeFile(fileNameToWriteToDisk, asyncApiFile, { encoding: 'utf8' });
console.log(`Created file ${fileNameToWriteToDisk}...`);
}
}