Configurable (symbol . value) bindings for file preview

[jdtsmith]

I have enabled inline images on load in org-mode, which is slow when previewing image-rich org files in consult-buffer. Right now my fix is to inhibit org startup during file preview, ala:

(defun my/consult-no-org-preview (fun)
  (lambda (&rest r)
    (let ((org-inhibit-startup t))
      (apply fun r))))
(advice-add #'consult--temporary-files :filter-return #'my/consult-no-org-preview)

This also prevents org-indent-mode from doing its thing. But this makes me think a user config like consult-preview-file-variables might be useful. It would be an alist of symbol names and values, like '((org-inhibit-startup . t)), which could be let-bound in the closure returned by consult--temporary-files using cl-progv or similar chicanery.

[astoff]

I just noticed that file previews will start Eglot, which is also slow. Maybe setting delay-mode-hooks would help for this case. But this is some shady chicanery indeed. (I don't think file previews should be on by default, TBH).

[jdtsmith]

I think for that you can (add-to-list 'consult-preview-excluded-hooks 'eglot--maybe-activate-editing-mode).

[astoff]

I wasn't aware of this option. Well, if something that's not a built-in in ever makes it into the default value of that variable, I guess Eglot is a good candidate.

(Also, to qualify my opinion that file previews should maybe be off by default, I'm specifically talking about consult-buffer here. consult-grep is different because in that case the live preview is very much a fundamental part of the functionality, while for buffer switching it's just a nifty extra.)

[jdtsmith]

Actually looking at the code, I'm not sure this (or its lsp-mode equivalent) is actually doing anything, other than removing such hooks from find-file-hooks (where they don't appear). I was quite sure that lsp-mode was firing before during preview; maybe @minad fixed this another way.

Update: looks like I fixed this myself using a consult-preview-p function (in my language server startup):

  (defun consult--preview-p ()
    (when-let (win (active-minibuffer-window))
      (not (eq nil (buffer-local-value
                    'consult--preview-function
                    (window-buffer win))))))
  :init
  (defun my/lsp-start-pyright ()
    (unless (consult--preview-p)
      (require 'lsp-pyright)
      (lsp-deferred)))
  :hook (python-mode . my/lsp-start-pyright))

I personally like the previews with consult-buffer too. I just want to "lightweight them" by turning off the especially heavy initialization bits (like contacting an LSP server or loading a bunch of images!).

[minad]

See also #515. Maybe the excluded hooks variable should be consulted for more hooks?

[chasecaleb]

See also #515. Maybe the excluded hooks variable should be consulted for more hooks?

Yeah, my needs (preventing LSP init) would be satisfied by checking the major mode hook. I assume you would have to either write some code to determine the major mode's hook variable, but assuming that's not too painful I think that would be a good solution. Alternatively you could make the user set a variable listing additional hooks to check, although that would be a minor nuisance because I have a dozen or so major modes that I use LSP mode with and would prefer not to have to keep my consult config in sync with that. Emphasis on minor nuisance though, I would be ok with either way.

[astoff]

I think the right thing to do here is to (cl-letf* (((default-value 'delay-mode-hooks t) ... when opening the temporary file.

It won't help specifically for org-inhibit-startup, but that seems to be a rather special case.

[jdtsmith]

I don't think that will help as much as could be hoped, because much of the initialization of a mode is done in the body of its mode function, not in its mode hooks. In things like lsp, much of the initialization is done in response to server contact, via the mode that starts the language server.

[astoff]

much of the initialization of a mode is done in the body of its mode function, not in its mode hooks

I don't think that's the typical situation. Besides org-mode, I'm aware of only one other mode for files that can be visibly slow to load, namely AUCTeX when TeX-parse-self is non-nil. (Well, there is code-cells, but that's a very niche thing.)

Anyway, what I suggested above is more specifically a solution for #515.

[minad]

See https://github.com/minad/consult/pull/542#issuecomment-1075611465 for a possible approach to handle delaying hooks and proper buffer initialization when entering recursive editing.

[minad]

In https://github.com/minad/consult/commit/3b0d745a480220d3eb8367fe73e8bf1321034a4e I implemented the approach outlined in https://github.com/minad/consult/pull/542#issuecomment-1075611465. This should ensure that file preview is much cheaper. The previewed file buffer is only fully initialized as soon as the user leaves the minibuffer window to do some recursive editing. Also all fully initialized buffers which have been visited by recursive editing are kept alive after quitting the minibuffer session. This is a change in behavior, since we only kept unmodified buffers alive before this change.

The method I used in https://github.com/minad/consult/commit/3b0d745a480220d3eb8367fe73e8bf1321034a4e is @astoff's sledgehammer from #542, where all hooks are delayed. Personally I am okay with this but it could be that for certain buffers the preview does not behave as expected. For example Org buffers are not nicely modernized by org-modern. ;)

I would appreciate if you can give it a thorough test!

[astoff]

I've noticed one weird interaction, where sometimes I get a prompt asking "Do you want to revisit the file normally now?" (you can find that string in files.el). I couldn't track down exactly when it happens, but so far only LaTeX files seem affected. I have a hunch that it might be related to latex-mode searching for \usepackage{inputenc} to decide on the file encoding.

It also happens on emacs -Q, so it's not AUCTeX-specific.

[minad]

@astoff That's odd since it means that the file is already open, which should not happen. Can you give me the steps needed to reproduce this? The Tex file must use inputenc?

[minad]

Since this change file preview got considerably faster. I am experimenting now in my config with generally enabling file preview. Hopefully this will help in uncovering further issues. See also some more tweaks in https://github.com/minad/consult/commit/2aaa9547b3d74b4bd6638c9821f6db76b8b1d6ba.

  (consult-customize
   consult-theme consult-ripgrep consult-git-grep consult-grep
   consult-bookmark consult-recent-file consult-xref
   consult--source-recent-file consult--source-bookmark
   consult--source-project-recent-file
   :preview-key '(:debounce 0.2 any))

[astoff]

@astoff That's odd since it means that the file is already open, which should not happen. Can you give me the steps needed to reproduce this? The Tex file must use inputenc?

I'm struggling to find a small reproducible example. But if you happen to have a directory full of tex files, you can try to search for ... with consult-grep and jump around until you get asked that question.

[minad]

In my tests I didn't see this and I also don't understand how that happens since the question should only be asked when the file is already open and then opened again with a different encoding. But in this case consult--temporary-files should not reopen the file. Instead it will reuse the existing buffer.

[jdtsmith]

Thanks for this @minad. For the issue mentioned here, setting delay-mode-hooks temporarily unfortunately isn't a fix. Many modes (including ones I've written!) do much of their initialization in the body of the mode definition. For example, org-mode does:

     (when org-startup-with-inline-images (org-display-inline-images))
     (when org-startup-with-latex-preview (org-latex-preview '(16)))
     (unless org-inhibit-startup-visibility-stuff (org-set-startup-visibility))
     (when org-startup-truncated (setq truncate-lines t))
     (when org-startup-numerated (require 'org-num) (org-num-mode 1))
     (when org-startup-indented (require 'org-indent) (org-indent-mode 1)))

Luckily it is wrapped by (unless org-inhibit-startup..., so I still need to set that for preview, as in the first post above. That's why I think an optional list of (somevar . bind-value) which the user could add as a supplement to your hard-coded let-wrapping:

                (cl-letf* (((default-value 'delay-mode-hooks) t)
                             ((default-value 'find-file-hook)

would still be valuable. But advice-wrapping the lambda returned by consult--temporary-files also still works.

[astoff]

Okay, with some debugging I traced down the error. It happens after the call to find-file-noselect in latexenc-find-file-coding-system. One way to avoid the error is by setting latexenc-dont-use-tex-guess-main-file-flag to t.

[minad]

@jdtsmith I think it is okay to initialize the modes. Is the initialization very expensive? For the org example you showed here, it doesn't seem to be the case. If you don't want any initialization at all, you can set consult-preview-raw-size to 0.

[jdtsmith]

The original issue is that I enable loading of images at org startup, so (org-display-inline-images) pulls up all sorts of PDFs and PNGs. Not too slow for small files/few images, but once the files gets large there is a delay. Similar to @astoff's case, the workaround is a let-bind-and-wrap a single variable away.

If I let-bind org-inhibit-startup as in my advice example above, only the expensive initializing gets skipped. You still get syntax highlighting, etc. I could also even just let-bind-wrap (org-startup-with-inline-images nil) to skip just this part and be fine. Just might be a bit easier for most users to have an easy consult-preview-temporary-variable-bindings list.

[minad]

@jdtsmith I don't like adding more and more such configuration options which require fine grained tuning. However we already set delay-mode-hooks. What you are proposing is a generalization of that, just allowing the user to set more variables around find-file-noselect, so that's not too bad. My main issue is not the addition of configuration options itself, but the fact that these require fine grained tuning. This is not something which many users would do. I prefer to have solutions which fix a bunch of issues at once.

[minad]

I looked at this again. We also bind many other variables locally (inhibit-message, non-essential and so on), so the generalization seems fine. I will give this a try.

[minad]

I added consult-preview-variables. Are there more variables we may want to set? I think it would be nice to preconfigure as much as possible here such that the preview is responsive for many different file types.

[jdtsmith]

Cool thanks. Trying it, it seems that the set is escaping the unwind-protect. E.g. org-startup-with-inline-images starts as t, but consult-buffer sets it to nil, and it remains nil afterwards.

[minad]

@jdtsmith Thanks. There were a few more issues with the current code. It should be fixed now.

[astoff]

I see one issue with the new consult-preview-variables. All variables that were unbound before the preview will remain bound (to nil) after the preview finishes. This won't cause any trouble with the default settings, since, luckily, all the vars mentioned there are either preloaded or have nil as default value.

But it the user adds to that list a variable that has a non-nil default and is not customized and then triggers some preview before the package defining the var is loaded, Emacs will be in an inconsistent state.

[minad]

@astoff Yes, I tried it the other way round before that, only binding variables which were bound before. See https://github.com/minad/consult/commit/38f0f39ce2e15ef353c6cb70435373e47cb9ae86. However this creates the inverse problem where libraries that are not yet loaded will get loaded by find-file-noselect with their variables, which will then have their undesired effects during preview. In order to fix this issue one has to set all the variables in the init.el before loading Consult.

EDIT: I took the more conservative approach now. Variables which are not defined are not bound and a warning is printed. Before adding a variable to consult-preview-variables, the variable must be initialized. This behavior makes sense since this is also what happens if one would use cl-letf on an undefined variable. Unfortunately this means that we cannot preconfigure many more variables than the ones we have already. I dropped the Org and Latexenc variables.

(cl-letf (((default-value 'foo) 42))
  nil)

[jdtsmith]

Have you tried cl-progv, i.e. set the variables dynamically only?

[minad]

@jdtsmith Yes of course I looked into cl-progv. Using let to bind variables would work however it seems to me that we have to temporarily set the default value of some of the variables. This requires unwind-protect and has the problematic consequence of leaving state around for variables which have not been initialized before. The same issue affects cl-letf, which is a pretty basic macro in Emacs! I don't think we can do much better unfortunately.

(cl-letf (((default-value 'var) :value))
  :body)

--->

(let*
    ((old
      (default-value 'var)))
  (unwind-protect
      (progn
        (set-default 'var :value)
        :body)
    (set-default 'var old)))

[minad]

If a variable is defined buffer-locally, then we have to use set+set-default+unwind-protect to ensure that the variable is overwritten correctly. In that case the variable is defined, no danger of the inconsistent state mentioned by @astoff.

1. If the variable is not defined buffer-locally (or not defined at all), then we could simply let bind it. However when the variable gets then initialized globally by find-file-noselect, our let binding will undo it. Then we get the inconsistency.

2. Alternatively if the variable is not defined, we do not bind it. Then we get no inconsistency, but if the variable gets initialized by find-file-noselect it may have evil effects which we want to avoid during preview.

3. Alternatively we also use set+unwind-protect. Then we could either unbind the variable again when unwinding. This would then lead to inconsistency if the variable got initialized by find-file-noselect. Or we just reset the variable to nil when unwinding, which also leads to an inconsistency.

It seems to me there is no other solution than requiring the variables to be defined before preview. :shrug: