jupyter icon indicating copy to clipboard operation
jupyter copied to clipboard

REPL fontification not working when using global-tree-sitter-mode

Open timlod opened this issue 3 years ago • 7 comments

Hi,

When using emacs-tree-sitter globally (global-tree-sitter-mode), there is no syntax highlighting in jupyter-repl-mode.

I believe the reason lies in jupyter-repl-initialize-fontification - font-lock-defaults without tree-sitter has this section (sorry about the escape sequences):

(font-lock-fontify-region-function .
                                    #[128 "\302\300\303\301\"\"\207"
                                          [jupyter-repl-font-lock-fontify-region
                                           (font-lock-default-fontify-region)
                                           apply append]
                                          6 "\n\n(fn &rest ARGS2)"])

However, when tree-sitter is enabled it will insert some tree-sitter code as such:

 (font-lock-fontify-region-function .
                                    #[128 "\302\300\303\301\"\"\207"
                                          [jupyter-repl-font-lock-fontify-region
                                           (#[128 "\300\301\"\207"
                                                  [apply tree-sitter-hl--highlight-region
                                                         #[128 "\301\302\300!\"\207"
                                                               [font-lock-fontify-region-function apply default-value]
                                                               4 "\n\n(fn &rest ARGS)"]
                                                         nil]
                                                  4 nil])
                                           apply append]
                                          6 "\n\n(fn &rest ARGS2)"])

I don't fully understand why, but I suppose that even though there is no highlighting for jupyter-repl-mode, it is looking at how fontification would work inside the jupyter-repl-lang (in this case python) buffer, and modifying the font-lock accordingly. However, since tree-sitter-hl-mode doesn't use font-lock, there won't be any syntax highlighting.

I'm OK with not using tree-sitter-hl-mode here, but would want at least the default python highlighting, so as a band-aid this works:

(defun jupyter-repl-no-tree-sitter (orig-fun &rest args)
  (global-tree-sitter-mode -1)
  (apply orig-fun args)
  (global-tree-sitter-mode 1))
(advice-add 'jupyter-repl-initialize-fontification :around #'jupyter-repl-no-tree-sitter)

Is there a way to get the two packages to work together natively? That would enable semantic highlighting from within Jupyter repl buffers, which would be pretty great.

By the way, jupyter-python src-blocks within org are already highlighted correctly with tree-sitter.

timlod avatar Nov 27 '21 12:11 timlod

The REPL fontification initialization just copies over the font-lock-defaults of the jupyter-repl-lang buffer with an updated version of font-lock-fontify-region-function that calls the version of that function from the jupyter-repl-lang buffer on the appropriate regions to be fontified in the REPL buffer, i.e. the input cells. So in this case since tree-sitter-mode is enabled in the jupyter-repl-lang buffer, the REPL buffer uses the tree-sitter override of font-lock-fontify-region-function to try and highlight the input cells.

Just browsing the tree-sitter code briefly, it looks like what is going on is that the internal variables that tree-sitter depends on, like tree-sitter-tree, are nil for the REPL buffer since it's not in tree-sitter-mode and therefore the call to tree-sitter-hl--highlight-region doesn't do any highlighting.

It looks like we need to have tree-sitter parse the input cell regions and set those variables in the REPL buffer before attempting to fontify an input cell. An input cell gets fontified at the following line

https://github.com/nnicandro/emacs-jupyter/blob/57306bf38512aea9462822a3c352f99f8d25f3d3/jupyter-repl.el#L1769

nnicandro avatar Nov 28 '21 04:11 nnicandro

As an alternative solution, one may wish to try this instead:

(use-package tree-sitter-langs
  :demand t
  :after tree-sitter
  :straight (tree-sitter-langs :host github
                               :repo "ubolonton/emacs-tree-sitter"
                               :files ("langs/*.el" "langs/queries"))
  :hook ((c++-mode python-mode sh-mode json-mode) . tree-sitter-hl-mode))

(use-package tree-sitter-langs
  :straight nil
  :after jupyter-repl
  :config (add-to-list 'tree-sitter-major-mode-language-alist '(jupyter-repl-mode . python))
  :hook (jupyter-repl-mode . tree-sitter-hl-mode))

I know that global-tree-sitter-mode is nice, but absolutely not necessary since tree-sitter is really only meaningful for a handful of languages/modes.

dangom avatar Nov 29 '21 02:11 dangom

I've tried this, but it runs into performance issues: https://github.com/emacs-tree-sitter/elisp-tree-sitter/issues/78.

Further, I was not able to get python tree-sitter highlighting in org-mode without global-tree-sitter-mode. In general, the global mode is really fine, as it's supposed to work 'whenever possible', and not do anything otherwise. I think this particular instance is an edge case.

timlod avatar Nov 29 '21 08:11 timlod

Thank you, nice investigation and very clear explanation on the thread. Also clarifies why I too sometimes see slowdowns after a using the REPL for a while. I thought it was somehow related to remote kernels.

dangom avatar Nov 29 '21 16:11 dangom

Thanks for pointing me in the direction of that issue. I'll make a comment to see if we can make progress on it.

nnicandro avatar Nov 30 '21 21:11 nnicandro

I'm adding a workaround in tree-sitter-hl: https://github.com/emacs-tree-sitter/elisp-tree-sitter/pull/201. Please check it out to see whether it works.

ubolonton avatar Jan 06 '22 16:01 ubolonton

I just tried it and on first glance it now works as expected without workarounds. I'll close this issue once you merge the commit - thanks already!

https://github.com/emacs-tree-sitter/elisp-tree-sitter/pull/201 (added link just to have GitHub reference this comment)

timlod avatar Jan 06 '22 16:01 timlod