company-reftex
company-reftex copied to clipboard
Add option to disable `reftex-parse-all`
Overview
This PR addresses #11 by adding an option to disable the call to reftex-parse-all
, which could be the cause of the performance sink reported.
Tasks before merge
- [x] Check if this PR addresses the performance issues reported in #11
- [ ] Check if the optimizations here further improve performance. If so, we could add this link to our readme so other users could add these to their emacs configuration
Thank you for that improvement.
Side note: I personally think a hook would have been better. You could have offered reftex-parse-all
and reftex-parse-one
as options for that hook but also give the user the possibility to use his own parser or, maybe, no parser at all. (Please don't be offended. That is just an opinion. I am happy about your efforts even if my approach would have been slightly different!)
I ask the OP to test your changes. (I do not have his LaTeX sources.)
I ask the OP to test your changes. (I do not have his LaTeX sources.)
Done: https://emacs.stackexchange.com/questions/73745/company-reftex-slow-when-searching-for-labels?noredirect=1#comment120306_73745
You could have offered reftex-parse-all and reftex-parse-one as options for that hook but also give the user the possibility to use his own parser or, maybe, no parser at all. (Please don't be offended. That is just an opinion. I am happy about your efforts even if my approach would have been slightly different!)
No problem and thanks for the clarification. Does #14 reflect the idea you had in mind?
In case I misunderstood something, it would be good if you could add a code suggestion so I can visualize this better.
Side note: I personally think a hook would have been better.
I see your point. The only thing I am hesitant about in regards to the hook is the (perhaps unnecessary) freedom provided to the user. AFAIK there are basically two ways to go about parsing the labels; either parse the single file or parse the multi-file document. Leaving this hook empty would render the parser useless. I also don't see any practical optimization that a user can make without going deep into reftex
and patching stuff there (if there is anything to be patched at all). But if a user was able to do that, then it makes more sense to make upstream changes in reftex
than company-reftex
since we simply use the bindings available to us from reftex
.
Do you know what I mean, or do you have another perspective?
I ask the OP to test your changes. (I do not have his LaTeX sources.)
Perfect, thank you.
Hello everybody, I just wanted to tune in here on the problem. I implemented the change Tobias asked me for and tried it on my system. Sadly, the problem still persists. As soon as I enter a \ref{}
command with the point; the autocomplete scans all my files for the Latex project.
UPDATE: The proposed changed did solve my problem. I did not recompile the package correctly. With the implemented changes all files still get scanned, but only once. Afterwards the performance is way better and the autocomplete suggestions turn up faster. This is a nice solution for my problem, thank you!
Hi @Captn-LootALot. Thanks for reporting back regarding the performance. One thing to clarify though:
This is a nice solution for my problem
While setting company-reftex-labels-parse-all
to nil
does solve your current problem, this could introduce a new problem as originally described in https://github.com/TheBB/company-reftex/issues/11#issue-831685128. With this option being set to nil
, if you change a label in one LaTeX file A and autocomplete \ref{}
in a LaTeX file B (assuming they are part of a larger document); you will no longer (automatically) have access to the changed label in LaTeX file A. Whereas with company-reftex-labels-parse-all
being t
, this multifile document parsing would happen automatically. Just wanted to state this as a FYI.
Afterwards the performance is way better and the autocomplete suggestions turn up faster.
Overall, I am on board to merge this PR as it constitutes a trivial but effective change. As mentioned in https://github.com/TheBB/company-reftex/pull/13#issuecomment-1264736701, I am less comfortable with having a customizable hook unless @TobiasZawada or @TheBB have strong opinions in favour of the customizable hook?
PS: I created a new PR #14 for the customizable hook so we keep the diffs in separate branches
Hello @atreyasha thank you for the heads-up. Whenever I need to scan all documents again I usually call company-reftex-labels
inside my Emacs editor. This works just fine.
I think there is a misunderstanding here.
Whenever I need to scan all documents again I usually call company-reftex-labels inside my Emacs editor
This PR will change how company-reftex-labels
works, such that calling company-reftex-labels
with the company-reftex-labels-parse-all
variable being nil
will no longer scan all ~documents~ files in a document. So the behaviour you mentioned that "works just fine" will no longer do the same thing post-merge. Below is a MWE to verify my point.
MWE
Environment (environment.tar.gz)
init.el
;;------------------------------------------------------------------------------
;; use-package and quelpa
;;------------------------------------------------------------------------------
;; setup archives
(require 'package)
(setq package-user-dir (file-name-directory (or load-file-name (buffer-file-name))))
(add-to-list 'package-archives '("gnu" . "https://elpa.gnu.org/packages/"))
(add-to-list 'package-archives '("melpa" . "https://melpa.org/packages/"))
(package-initialize)
;; setup use-package
(unless (package-installed-p 'use-package)
(package-refresh-contents)
(package-install 'use-package))
(eval-and-compile
(setq use-package-always-ensure t
use-package-expand-minimally t))
;; setup quelpa
(unless (package-installed-p 'quelpa)
(with-temp-buffer
(url-insert-file-contents "https://raw.githubusercontent.com/quelpa/quelpa/master/quelpa.el")
(eval-buffer)
(quelpa-self-upgrade)))
(quelpa '(company-reftex :fetcher git
:url "https://github.com/TheBB/company-reftex"
:branch "as/parse-all-defcustom"))
;;------------------------------------------------------------------------------
;; company and reftex
;;------------------------------------------------------------------------------
;; enable RefTex in emacs latex mode
(add-hook 'latex-mode-hook 'turn-on-reftex)
;; install and enable company-reftex
(use-package company
:config
(setq company-idle-delay nil))
(global-company-mode t)
(use-package company-reftex)
;; disable reftex-parse-all for labels
(setq company-reftex-labels-parse-all nil)
;; add new backend
(add-to-list 'company-backends 'company-reftex-labels)
main.tex
\documentclass{article}
\begin{document}
\include{file_1}
\include{file_2}
\end{document}
%%% Local Variables:
%%% mode: latex
%%% TeX-master: t
%%% End:
file_1.tex
\label{Chapter 1}
%%% Local Variables:
%%% mode: latex
%%% TeX-master: "main"
%%% End:
file_2.tex
\label{Chapter 2}
%%% Local Variables:
%%% mode: latex
%%% TeX-master: "main"
%%% End:
Directory structure for latex files
.
├── file_1.tex
├── file_2.tex
└── main.tex
0 directories, 3 files
Steps to reproduce
-
Launch
emacs
using the aforementioned simplifiedinit.el
viaemacs -q -l /path/to/simplified/init.el
.Note: This will dump installed packages in the same directory as
init.el
, so it makes sense to place thisinit.el
outside of~/.emacs.d/
-
Open
file_1.tex
andfile_2.tex
-
Open the buffer for
file_1.tex
and executecompany-reftex-labels
with the cursor inside\ref{}
in a new line. You should see autocompletions forChapter 1
andChapter 2
:\label{Chapter 1} \ref{} ;; cursor inside the curly brackets %%% Local Variables: %%% mode: latex %%% TeX-master: "main" %%% End:
-
Add a new line in
file_2.tex
containing\label{foo}
and save this file:\label{Chapter 2} \label{foo} %%% Local Variables: %%% mode: latex %%% TeX-master: "main" %%% End:
-
Repeat step 3. You should see autocompletions for
Chapter 1
andChapter 2
, but not forfoo
-
Evaluate
(setq company-reftex-labels-parse-all t)
-
Repeat step 3. You should see autocompletions for
Chapter 1
,Chapter 2
andfoo
Hope this clarifies. I will wait for responses from @TobiasZawada and/or @TheBB before merging.
Thank you for coming back to this. I tested your MWE and you are absolutely right. I do understand now that what I earlier wrote cannot be possible. I will take a look at my earlier setup and see what I did wrong. Again, thank you!
Thanks @atreyasha, this is a nice fix. I often work with multi-file latex projects, and this PR improves performance drastically. I just call reftex-parse-all
when I change a label in another file (which is not so common, so the performance increase is really worth the tradeoff).