search

James-Yu / LaTeX-Workshop

Boost LaTeX typesetting efficiency with preview, compile, autocomplete, colorize, and more.
MIT License
10.55k stars 522 forks source link

SyncTeX binary precedence over synctex.js? #4186

Closed James-Yu closed 6 months ago

James-Yu commented 6 months ago

[Internal Discussion]

As per https://github.com/James-Yu/LaTeX-Workshop/issues/4181#issuecomment-1974758362 and #4168, I did not find the column-locating code for synctexjs we have been using. I also backtrace a few years code in the past and did not find that. Strangely both you @jlelong and I seem to have an impression on the feature.

I also checked (roughly) the code on invoking SyncTeX binary and figured that the binary indeed returns with a column index: https://github.com/James-Yu/LaTeX-Workshop/blob/6c63cc3e64f782a6dbba0d2e98ec5ebe5b927878/src/locate/synctex.ts#L98-L101 instead of the 0-column of synctexjs: https://github.com/James-Yu/LaTeX-Workshop/blob/6c63cc3e64f782a6dbba0d2e98ec5ebe5b927878/src/locate/synctex/worker.ts#L236 But now we have latex-workshop.synctex.synctexjs.enabled defaults to true: https://github.com/James-Yu/LaTeX-Workshop/blob/6c63cc3e64f782a6dbba0d2e98ec5ebe5b927878/package.json#L1682-L1687

What about we return to first try the binary, if anything went wrong (not found, non-zero return), we fallback to the javascript implementation to balance the utility and availability? We may also remove this config if so, or change the purpose of it to be overriding this new logic. May I have your thoughts? @jlelong

jlelong commented 6 months ago

I have good news: we are not crazy yet! I have managed to trace back to the column-locating code. It was introduced in #1161. It is still alive

https://github.com/James-Yu/LaTeX-Workshop/blob/ee13477a5a2bf94ed107f3de2572aa55f96423c0/src/locate/synctex.ts#L471-L492

Ctrl-clicking inside the pdf calls https://github.com/James-Yu/LaTeX-Workshop/blob/614eec118aa95a390237d0ec60fd15d7edb2b18d/src/preview/viewer.ts#L300

If I understand correctly, synctex does provide a column number but it can be quite inaccurate. The function getRowAndColumn tries to fix it.

James-Yu commented 6 months ago

Thats awesome! I will look into the implementation c.f. #4181 #4168