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

Stop stripping mainFiles extension before build #89

Merged
merged 4 commits into from
Jul 30, 2017
Merged

Stop stripping mainFiles extension before build #89

merged 4 commits into from
Jul 30, 2017

Conversation

simon123h
Copy link
Contributor

@simon123h simon123h commented Jul 29, 2017

As of now, the extension of the main file is stripped before compilation. Thus, example.tex will result in a command like pdflatex example.
This works fine for .tex files, as pdflatex will search for both example and example.tex. However it fails at custom extensions, like .tikz.

This commit removes the stripping of the main files extension. So, example.tikz would be passed to pdflatex as it is.

Related issue: #86

@James-Yu
Copy link
Collaborator

Thank you for your contribution. Unfortunately, the PR cannot be merged as is. This is because that the change breaks its compatibility with bibtex, which can only accept main but not main.tex.

In https://github.com/James-Yu/LaTeX-Workshop , the suggested solution is to use project-specific configuration. AFAIK Atom is still developing the feature, so an alternative could be add a new placeholder.

@simon123h
Copy link
Contributor Author

I had a quick glance at https://github.com/James-Yu/LaTeX-Workshop: LaTeX-Workshop strips only .tex extensions. I adopted their implementation. This way, compilation works for all filetypes, while bibtex is still broken for non-.tex files.
Anyways, this is an improvement over compilation being generally broken for all types.

Also I reverted the TikZ extension commit, as it can be discussed in it's own PR #90.

@James-Yu James-Yu merged commit 5ae8f2c into ashthespy:master Jul 30, 2017
@ashthespy
Copy link
Owner

ashthespy commented Aug 3, 2017

Was just having a feeling of Déjà vu, wondering why the regex was there in the first place.

By stripping only .tex extensions, #42 will be broken again - that was when f35d4bc5e524ebf50866d96076fc09a68532ef95 was implemented. With this PR, we are basically reverting back to that right?

Wouldn't it better to introduce a new placeholder such that %DOC is always just the base filename, and %EXT is the extension that can be used for people who want finer control?

@simon123h
Copy link
Contributor Author

@ashthespy as James mentioned before, introducing a new placeholder would be a good solution. I was thinking of %DOC for the document with extension and %NAME for the document name without extension. Though, I like prefer it the way you propose it with %DOC and %EXT.
It is important not to break existing custom toolchains. Your idea is consistent with the previous behavior. Bravo!

@ashthespy
Copy link
Owner

ashthespy commented Aug 10, 2017

Sounds like a good compromise right?

cmd = cmd.split('%DOC').join(
        # get basename and strip file extensions
        @escapeFileName(path.basename(@latex.mainFile).replace(/\.([^\/]*)$/, ''))
      )
cmd = cmd.split('%EXT').join(path.basename(@latex.mainFile).match(/\.([^\/]*)$/)[1])

Should also handle the odd case that someone wants to get funky with a main.tex.tikz ;-)

@simon123h would you be able to implement this or should I?

Edit: I had a few minutes - It's done 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants