Fixes #46 #99

sunwukonga · 2020-04-03T23:28:08Z

Run tex engine before and after bib executable
Add support for both bibtex and biber

Add global variable g:livepreview_biber
Add Bibliography section to README
Detail usage of g:livepreview_biber
Detail usage with biblatex

Summary

Provide a general description of the code changes in your pull request... were there any bugs you had fixed? If so,
mention them. If these bugs have open GitHub issues, be sure to tag them here as well, to keep the conversation linked
together.

Other Information

If there's anything else that's important and relevant to your pull request, mention that information here. This could
include benchmarks, or other information.

If you are updating any of the CHANGELOG files or are asked to update the CHANGELOG files by reviewers, please add the
CHANGELOG entry at the top of the relevant section.

Thanks for contributing to vim-latex-live-preview!

badouralix

Hi @sunwukonga !

Welcome and thanks a lot for your contribution 🎉
This has been a very long running problem and your solution is more than welcome !

I have added a few design comments that I think could help, let me know what you think about them

badouralix · 2020-05-08T19:31:23Z

plugin/latexlivepreview.vim

+        if exists('g:livepreview_biber') && g:livepreview_biber > 0
+            let s:bibexec = 'biber *.bcf'
+        else
+            let s:bibexec = 'bibtex *.aux'
+        endif


Other configuration flags ( engine, previewer ) can set the binary directly, we could try to do something similar for bibliography

Suggested change

if exists('g:livepreview_biber') && g:livepreview_biber > 0

let s:bibexec = 'biber *.bcf'

else

let s:bibexec = 'bibtex *.aux'

endif

if (s:bibengine == 'biber')

let s:bibexec = 'biber ' . l:tmp_root_dir . '/*.bcf'

else

let s:bibexec = s:bibengine . ' ' . l:tmp_root_dir . '/*.aux'

endif

We should also set this in the Initialize function ( found this in https://stackoverflow.com/a/33775128, to be tested though )

" Get the bib engine let s:bibengine = get(g:, 'livepreview_bibengine', 'bibtex')

vim-latex-live-preview/plugin/latexlivepreview.vim

Lines 230 to 252 in 855c309

" Get the tex engine

if exists('g:livepreview_engine')

let s:engine = g:livepreview_engine

else

for possible_engine in ['pdflatex', 'xelatex']

if executable(possible_engine)

let s:engine = possible_engine

break

endif

endfor

endif

" Get the previewer

if exists('g:livepreview_previewer')

let s:previewer = g:livepreview_previewer

else

for possible_previewer in ['evince', 'okular']

if executable(possible_previewer)

let s:previewer = possible_previewer

break

endif

endfor

endif

I went back and forth on this, but I finally settled on disagreeing with this suggested change for the following reasons:

pdflatex and xelatex are drop in replacements of each other, as are evince and okular, however bibtex and biber have usages divergent enough to break things. They will be a binary choice for the foreseeable future.

I don't think we should ask the user to spell something when they can simply select instead.

I did however use get() with some added validation for both g:livepreview_engine and g:livepreview_previewer which now gives a warning if no valid executable is available for either of those roles.

badouralix · 2020-05-08T19:32:40Z

plugin/latexlivepreview.vim

+                \ ' && ' .
+                \       b:livepreview_buf_data['run_cmd'] .
+                \ ' && ' .
+                \       'cd ' . l:tmp_root_dir . ' && ' . s:bibexec .


We should try to avoid using cd as much as possible, as this could lead some unintended behaviors and makes the code quite hard to reason about

Agree. It's bitten me more than once on other scripts. However, I have a memory that using a path to reference the file failed for one of the bib executables.

Eventually, I wasn't able to find a way around cd like behaviour with bibtex because it doesn't allow writing to external directories without a special option set outside the purview of vim-llp.

badouralix · 2020-05-08T19:32:58Z

plugin/latexlivepreview.vim

        " Update compile command with bibliography
        let b:livepreview_buf_data['run_cmd'] =
                \       'env ' .
                \               'TEXMFOUTPUT=' . l:tmp_root_dir . ' ' .
                \               'TEXINPUTS=' . l:tmp_root_dir
                \                            . ':' . b:livepreview_buf_data['root_dir']
                \                            . ': ' .
-                \       'bibtex ' . l:tmp_root_dir . '/*.aux' .
+                \ ' && ' .
+                \       b:livepreview_buf_data['run_cmd'] .


The inflight run_cmd is already being called once previous to this point in code, and it is called again a few lines below, do we need it here too ?

vim-latex-live-preview/plugin/latexlivepreview.vim

Line 171 in 855c309

silent call system(b:livepreview_buf_data['run_cmd'])

On review, this was all kind of a mess. When I first submitted the PR, I made the simplest and easiest changes I could without upsetting the surrounding code.

After your comment, I ripped out a section of code and replaced it instead. Which cleans this up.

README.md

sunwukonga · 2020-05-10T23:02:08Z

I've made changes over multiple commits that reduce the clarity of the conversation to date. With your permission, I'd like to squash all the commits and --force push to clean it up. Then you'll have a clean slate to comment upon; plus the commit history will be cleaner.

Old comments only persist if they were made on the PR directly or on the files changed view; otherwise if the comment was made on a commit hash that is subsequently changed by the force, it is lost.

An alternative is for me to create a new branch and squash on it, and reference this PR in a new one. Then this PR can be rejected; but the conversation will remain intact, and a new conversation can begin on the new PR.

badouralix

Yeah yeah yeah, feel free to do whatever you need to get a clean and clear history
I'm totally fine with force pushes, we can keep this PR
( added a few nitpicks in the meantime )

badouralix · 2020-05-14T20:29:54Z

README.md

+`biber`, but uses `biber` by default. To use `bibtex`, add `backend=bibtex`
+to your `biblatex` usepackage declaration.
+
+```


Suggested change

```

```latex

badouralix · 2020-05-14T20:32:59Z

README.md

+If you're using `biblatex` this is most likely caused by not also setting 
+`g:livepreview_use_biber = 1` in your `.vimrc`. Or if you intended to use
+`bibtex` not using that option when using the `biblatex` package. i.e.
+```


Suggested change

```

```latex

badouralix · 2020-05-14T20:35:13Z

plugin/latexlivepreview.vim

+        let l:matches = matchstr(getline(l:position), specific_pattern)
+        if ( l:matches == '' )
+            " expect s:useBiber=1
+            if ( s:useBiber == 0 )


Let's use snake case here instead of camel case, in order to keep it consistent with other variables

Run tex engine before and after bib executable Add support for both `bibtex` and `biber` - Add global variable `g:latexlivepreview_use_biber` Add Bibliography section to README - Detail usage of `g:latexlivepreview_use_biber` - Detail usage with `biblatex` Clean up calls to bibexec commands Remove `cd` from processing of bib commands - Move code that selects this option to the initializer Give more meaningful feedback when `engine` and `preview` commands are not valid - Throw errors from this context Catch errors in the initializer and then stop execution Move README section about bibliographies Update README to reflect change of g:latexlivepreview_biber Add section in Known Issues about bibliographies to README Add auto handling of certain bib setting mismatches Fix bugged warning when `biblatex` not used Add latex label to latex code in README Use snake_case not camelCase

badouralix

Thanks a lot for your contribution 🎉

badouralix reviewed May 8, 2020

View reviewed changes

badouralix added the bug label May 8, 2020

badouralix reviewed May 14, 2020

View reviewed changes

sunwukonga force-pushed the master branch from c26c136 to b3dfda5 Compare May 19, 2020 23:56

badouralix approved these changes Jun 25, 2020

View reviewed changes

badouralix merged commit f173602 into xuhdev:master Jun 25, 2020

ayazhafiz mentioned this pull request Sep 9, 2020

open -a Preview does not work as a live preview command anymore #108

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #46 #99

Fixes #46 #99

sunwukonga commented Apr 3, 2020

badouralix left a comment

badouralix May 8, 2020 •

edited

Loading

sunwukonga May 9, 2020

sunwukonga May 9, 2020

badouralix May 8, 2020

sunwukonga May 8, 2020 •

edited

Loading

sunwukonga May 9, 2020

badouralix May 8, 2020

sunwukonga May 9, 2020

sunwukonga commented May 10, 2020

badouralix left a comment

badouralix May 14, 2020

badouralix May 14, 2020

badouralix May 14, 2020

badouralix left a comment

	" Get the tex engine
	if exists('g:livepreview_engine')
	let s:engine = g:livepreview_engine
	else
	for possible_engine in ['pdflatex', 'xelatex']
	if executable(possible_engine)
	let s:engine = possible_engine
	break
	endif
	endfor
	endif

	" Get the previewer
	if exists('g:livepreview_previewer')
	let s:previewer = g:livepreview_previewer
	else
	for possible_previewer in ['evince', 'okular']
	if executable(possible_previewer)
	let s:previewer = possible_previewer
	break
	endif
	endfor
	endif

Fixes #46 #99

Fixes #46 #99

Conversation

sunwukonga commented Apr 3, 2020

Summary

Other Information

badouralix left a comment

Choose a reason for hiding this comment

badouralix May 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunwukonga May 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunwukonga commented May 10, 2020

badouralix left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

badouralix left a comment

Choose a reason for hiding this comment

badouralix May 8, 2020 •

edited

Loading

sunwukonga May 8, 2020 •

edited

Loading