alabid / flylatex

FlyLatex: A Realtime Collaborative Environment (with a concurrent editor) in node-js.
766 stars 85 forks source link

Editable tempdir, pdflatex and persistent tempfile fix. #10

Closed aspann closed 11 years ago

aspann commented 11 years ago
aspann commented 11 years ago

I know that, so if you leave it blank, it will use your system settings.

the main problem i thought was the persistent logflie creation and the non absolute path to pdflatex.

alabid commented 11 years ago

Hi @zappel2k, I saw your pull request. Thanks for taking the time to do this.

But I have the following issues with it: 1) Some changes to the bootstrap CSS were made. I'd rather not change any "included libraries" (one of which is bootstrap) because these libraries work cross-platform. I think your CSS changes were very specific to your platform. 2) I think that adding a temp config is absolutely unnecessary. See the comment I made in the "diff". 3) You changed MAX_DOCS from 20 to 100. This seems like it should be configurable at the top-level. So instead of changing this in routes.js, we can instead add a new variable MAX_DOCS. 4) Your "fixing pdflatex path" seems good but your "fixing TEMP path" is incorrect. First, because there's a misspelling as you check for .length. Second, I don't see why you want to make temp path a relative path... This is more reason why the temp config variable is not needed.

On the other hand, these look good: 1) I think adding a variable for running the pdflatex exec is a really good idea. 2) Adding "-output-directory=" to the pdflatex execute string

I cannot merge your pull request until these issues are fixed.

aspann commented 11 years ago

Gimme a sec.. ;)