stevedlawrence commented on code in PR #107:
URL: https://github.com/apache/daffodil-vscode/pull/107#discussion_r867926680
##########
src/infoset.ts:
##########
@@ -68,6 +68,10 @@ export async function activate(ctx: vscode.ExtensionContext)
{
)
ctx.subscriptions.push(
vscode.debug.onDidTerminateDebugSession(async (s) => {
+ if (sid !== undefined) {
+ let path = rm(tmp(sid))
+ rm(`${path}.prev`)
Review Comment:
Super minor, but I went down a bit of a rabbit hole because I missed the rm
aroudn tmp(sid), and thought the main tmp file was deleted somehow else.
Thoughts on changing to this:
```node
let path = tmp(sid)
rm(`${path}`)
rm(`${path}.prev`)
```
I think that makes it a bit more clear that both files are being removed.
##########
src/infoset.ts:
##########
@@ -177,3 +181,10 @@ function ensure(path: string): string {
}
return path
}
+
+function rm(path: string): string {
+ if (fs.existsSync(path)) {
+ fs.rmSync(path)
Review Comment:
I'm trying to learn a little node. Instead of checking `existsSync` and then
removing, could just use the `force` option to `rmSync`?
```node
fs.rmSync(path, { force: true })
```
At which point, I wonder if this function provides that much value? Could
you just do something like this above:
```node
let path = tmp(sid)
fs.rmSync(`${path}`, { force: true })
fs.rmSync(`${path}.prev`, { force: true })
```
It's a bit more verbose, but very clear what's going on.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
