andrewrynhard / atom-latex-plus

16 stars 13 forks source link

Problems with symlink generation #40

Closed svkurowski closed 8 years ago

svkurowski commented 8 years ago

There are some problems with the current implementation of the symlink generation (which can be found here).

First, await does not work with the fs.symlink function as it's currently used. await only works with promises which means that the execution is continued immediately instead of waiting for the symlink to be created. This can be easily tested by putting console.log("a"); into the symlink callback and console.log("b"); right below the function call. Many times, b will be logged earlier than a. This problem can be fixed by wrapping the symlink function into a function returning a promise.

Next, the callback is wrong in that it doesn't check whether err is actually defined. The callback is a completion callback and not just an error callback, which means that when the symlink is successfully created, the callback will be called without passing a parameter. This means that currently cannot link undefined : null is logged to the console when the linking successful.

However, this behaviour was pretty much unnoticeable, because the symlinking fails basically everytime due to the symlink existing already. This means that upon building (except for the first time), a error like the following will be always logged:

cannot link undefined : Error: EEXIST: file already exists, symlink '.../.latex/main.synctex.gz' -> '.../main.synctex.gz'

This might seem irrelevant but if one builds using the plugin's .latex default output directory name, then changes the output to e.g. .build in the project settings, the symlink will not be updated ever (unless done by the user manually. This means the symlink points to the old build file (or nothing in case the user deleted the .latex folder).

The correct way to solve this, is in my opinion:

This behavior is more complex than the trivial solution (always unlink, then create the symlink) because:

  1. It avoids recreating the symlink when it's not necessary
  2. It never unlinks a users file or directory which he might have in this directory with the same name

I have fixed the issues with the proposed behavior in my fork and will open a pull request shortly.