marionebl / svg-term-cli

Share terminal sessions via SVG and CSS
MIT License
3.52k stars 118 forks source link

--profile breaks with absolute paths #21

Closed adius closed 6 years ago

adius commented 6 years ago

https://github.com/marionebl/svg-term-cli/blob/ac9422756330b8970974e84b309076b61f985408/src/cli.ts#L200

fs.exists is an antipattern and should not be used. Just try to load the file and if it doesn't work, handle the error.

Details at http://devdocs.io/node~8_lts/fs#fs_fs_exists_path_callback

fs.open('myfile', 'wx', (err, fd) => {
  if (err) {
    if (err.code === 'EEXIST') {
      console.error('myfile already exists');
      return;
    }

    throw err;
  }

  writeMyData(fd);
});

But the bigger problem is that you don't normalize the paths => Absolute file paths lead to an error.

You just have to wrap the passed argument with a path.resolve

$ node
> path.resolve('./like/what')
'/Users/adrian/Projects/like/what'
> path.resolve('/like/what')
'/like/what'
>

No fiddling with process.cwd() necessary

marionebl commented 6 years ago

Fixed via 2.1.1

adius commented 6 years ago

Uhm, not really. It still discards any errors and just assumes an error means that the file does not exist. And it could happen that the file gets deleted after you check if it exists and before it gets really opened.

marionebl commented 6 years ago

Understood - absolute paths work now, though. So the "primary" issue is solved.

I'll look into making this more solid sometime next week.