jdtsmith / indent-bars

Fast, configurable indentation guide-bars for Emacs
GNU General Public License v3.0
359 stars 14 forks source link

Interaction with lsp-semantic-tokens-mode #72

Closed brownts closed 2 weeks ago

brownts commented 3 weeks ago

I've been noticing some oddities lately with indent-bars-mode. I initially noticed this when I would perform a revert-buffer-quick command, that afterwards the font locking seemed to be broken. I've since narrowed this down to when I have semantic highlighting enabled in lsp-mode along with using indent-bars-mode. With this combination, a buffer revert seems to break the highlighting. However, I discovered an even easier test. If I have both of these enabled, when I disabled indent-bars-mode, it would remove the semantic highlighting from lsp-mode. I've attached a couple screen shots showing before and after disabling indent-bars-mode in a buffer where this occurs.

I quickly looked at lsp-semantic-tokens-mode and noticed that it installs advice around font-lock-fontify-region-function. I also see that indent-bars-mode sets this locally. I'm not sure if this is where the problem is, but it might be. I think the order of the modes being activated might also be important.

Screenshot_2024-09-30_12-53-19 Screenshot_2024-09-30_12-53-36

jdtsmith commented 3 weeks ago

Unfortunately I can't really see a difference in your screenshots other than bars missing. Or is it the "bold" sscanf (e.g.)?

When it is enabled, indent-bars saves the value of font-lock-fontify-region-function in the variable indent-bars--orig-fontify-region. What is the value of that? Does that function have advice on it? Does C-h f indent-bars--fontify indicate that function has advice?

My guess is what's happening is indent-bars is turned-on first, and inserts its own font-lock-fontify-region-function (namely indent-bars--fontify). lsp-mode is then turned-on, and advises that. So when indent-bars gets disabled, it restores the old (non-advised) function. Let me know if that checks out.

Probably the right order of operations to achieve what you want is:

  1. Turn on lsp-mode in the buffer — advises font-lock-default-fontify-region
  2. Turn on indent-bars in the buffer — wraps the now-advised font-lock-default-fontify-region
  3. Turn indent-bars off — restores the advised font-lock-default-fontify-region) & lsp-mode mode advice still works.

BTW, you may be a good candidate for the new ability for indent-bars-no-descend-lists to be set to a list of valid opening chars (e.g. ( and [ only). See #65.

jdtsmith commented 3 weeks ago

Also, to turn on indent-bars late, you can use the depth argument of add-hook and set it to a high value like 97.

brownts commented 2 weeks ago

Unfortunately I can't really see a difference in your screenshots other than bars missing. Or is it the "bold" sscanf (e.g.)?

Probably the most obvious is to look at variables when used as parameters. For instance look at "env" in all of the "jinx_defun" calls, they change from font-lock-variable-name-face (blueish) to the default foreground face (white).

When it is enabled, indent-bars saves the value of font-lock-fontify-region-function in the variable indent-bars--orig-fontify-region. What is the value of that?

font-lock-default-fontify-region

Does that function have advice on it?

No, at least C-x f font-lock-default-fontify-region doesn't report that it does.

Does C-h f indent-bars--fontify indicate that function has advice?

No, at least C-x f indent-bars--fontify doesn't report that it does.

However, it appears the variable font-lock-fontify-region-function has advice on it.

font-lock-fontify-region-function is a variable defined in ‘font-lock.el’.

Its value is
#f(advice lsp-semantic-tokens--fontify :around indent-bars--fontify)
Local in buffer jinx-mod.c; global value is 
font-lock-default-fontify-region

My guess is what's happening is indent-bars is turned-on first, and inserts its own font-lock-fontify-region-function (namely indent-bars--fontify). lsp-mode is then turned-on, and advises that. So when indent-bars gets disabled, it restores the old (non-advised) function. Let me know if that checks out.

For this experiment, lsp is in the c-ts-mode-hook and indent-bars-mode is in the hack-local-variables-hook (so it's enabled after any potential directory local or EditorConfig indent variable changes). I'd assume that lsp is being initialized before indent-bars-mode, since it's in the mode hook. However, I added some tracing to see which order things are being initialized. As you can see, even though lsp is started first, lsp-semantic-tokens-mode isn't started until later (likely due to a callback within lsp-mode after an event occurs).

(progn
  (trace-function #'lsp nil (lambda () (print (buffer-name))))
  (trace-function #'lsp-semantic-tokens-mode nil (lambda () (print (buffer-name))))
  (trace-function #'indent-bars-mode nil (lambda () (print (buffer-name)))))
======================================================================
1 -> (lsp)jinx-mod.c
1 <- lsp: #("LSP :: Connected to [clangd:1614115/starting /home/troy/.emacs.custom/elpaca/repos/jinx]." 0 3 (face success))jinx-mod.c
======================================================================
1 -> (indent-bars-mode)jinx-mod.c
1 <- indent-bars-mode: tjinx-mod.c
======================================================================
1 -> (indent-bars-mode)jinx-mod.c
1 <- indent-bars-mode: tjinx-mod.c
======================================================================
1 -> (lsp-semantic-tokens-mode 1)jinx-mod.c
1 <- lsp-semantic-tokens-mode: tjinx-mod.c

Probably the right order of operations to achieve what you want is:

  1. Turn on lsp-mode in the buffer — advises font-lock-default-fontify-region
  2. Turn on indent-bars in the buffer — wraps the now-advised font-lock-default-fontify-region
  3. Turn indent-bars off — restores the advised font-lock-default-fontify-region) & lsp-mode mode advice still works.

As per above, I'm not sure how easy that is to accomplish due to the lsp-semantic-tokens-mode being turned on asynchronously which appears to happen after indent-bars-mode.

When I turn off indent-bars-mode at this point (i.e., M-x indent-bars-mode), I notice that the advice around font-lock-fontify-region-function seems to have been removed by this process.

font-lock-fontify-region-function is a variable defined in ‘font-lock.el’.

Its value is ‘font-lock-default-fontify-region’
Local in buffer jinx-mod.c; global value is the same.

Function to use for fontifying a region.
It should take two args, the beginning and end of the region, and an optional
third arg VERBOSE.  If VERBOSE is non-nil, the function should print status
messages.  This is normally set via ‘font-lock-defaults’.
If it fontifies a larger region, it should ideally return a list of the form
(jit-lock-bounds BEG . END) indicating the bounds of the region actually
fontified.

  This variable may be risky if used as a file-local variable.

BTW, you may be a good candidate for the new ability for indent-bars-no-descend-lists to be set to a list of valid opening chars (e.g. ( and [ only). See #65.

Thanks, I'll have to take a look at that.

jdtsmith commented 2 weeks ago

You cannot add advice to a variable, but you can add it to a function a variable points to. That is what's happening here. Why don't you try removing indent-bars from any hook, then loading a file all the way through semantic-tokens-mode activating, and then enabling it by hand. The advised function should be font-lock-default-fontify-region prior to that, and your extra faces should remain after disabling indent-bars.

If this works, to have this happen automatically, you could perhaps enable indent-bars-mode in the lsp-semantic-tokens-mode-hook, in those modes where it will be active.

jdtsmith commented 2 weeks ago

It's also a bit dangerous for lsp to directly advise a function which other modes are encouraged to replace as needed. Rather than advice, it would be better for it to wrap and replace that function (as indent-bars does); that's the reason an indirection variable is provided. Then either load order would work (though only the last can safely be disabled without affecting the other). indent-bars is not the only mode to update font-lock-fontify-region-function.

brownts commented 2 weeks ago

You cannot add advice to a variable, but you can add it to a function a variable points to. That is what's happening here.

Yeah, it just seemed weird that the advice was shown in the variable information instead of the function information.

Why don't you try removing indent-bars from any hook, then loading a file all the way through semantic-tokens-mode activating, and then enabling it by hand. The advised function should be font-lock-default-fontify-region prior to that, and your extra faces should remain after disabling indent-bars.

If this works, to have this happen automatically, you could perhaps enable indent-bars-mode in the lsp-semantic-tokens-mode-hook, in those modes where it will be active.

Possibly, but this all seems very brittle. When I revert the buffer C-x x g, lsp-semantic-tokens-mode is disabled and then is re-enabled. It's possible, that by adding indent-bars-mode into the lsp-semantic-tokens-mode-hook, I might be able to get this to work consistently. If feels like a house of cards, however.

It's also a bit dangerous for lsp to directly advise a function which other modes are encouraged to replace as needed. Rather than advice, it would be better for it to wrap and replace that function (as indent-bars does); that's the reason an indirection variable is provided. Then either load order would work (though only the last can safely be disabled without affecting the other). indent-bars is not the only mode to update font-lock-fontify-region-function.

I'm fine advocating for a change on the lsp-mode side, but I don't think the limitation that you can only disable the last one is sufficient (in a wrapper based implementation), based on the observation of the behavior during a buffer revert where it's really difficult to strictly control the order that modes are enabled/disabled. I think it's possible to make the wrapper approach work (although maybe there's a better solution). I know very little about font-lock, but I'm surprised there isn't some kind of local hook that modes could just add and remove themselves from for this, as that would seem to solve this problem.

However, if a wrapper-based approach must be used, I think you need to at least follow the following rules so that any mode can be disabled, but outer wrapped modes still function:

  1. During mode enable, only set font-lock-fontify-region-function with your mode-specific "fontify" function if your mode-specific "original" buffer-local variable is nil. (This allows re-enabling your mode after it was disabled, but you weren't the outer wrapper when the mode was disabled).
  2. During mode disable, only replace your mode-specific "fontify" function with "original" if your "fontify" function is still what is in font-lock-fontify-region-function. If you restored "original", set your mode-specific "original" buffer-local variable to nil. (This allows only restoring if you were the outermost wrapper).
  3. During execution of your "fontify" function, only perform your mode-specific fontification if your mode is enabled. (This allows your "fontify" function to act as a pass-through when you weren't able to remove it during mode disable).
  4. During execution of your "fontify" function, if your mode is disabled, check if your "fontify" function is what is in font-lock-fontify-region-function, and if it is, perform step 2. (This removes your mode as the outer-most wrapper since you couldn't remove it when your mode was previously disabled).
jdtsmith commented 2 weeks ago

I already do most of these. Will take another look. I can't perform too many checks in the function itself as it is called frequently. On revert the mode is not first disabled, correct?

In terms of API, there is jit-lock-functions, but no suitable way for multiple back ends working on the same properties (e.g. face) to work cooperatively. If interested, see this bug, with lots of details on how this works in indent-bars.

jdtsmith commented 2 weeks ago
  1. During mode enable, only set font-lock-fontify-region-function with your mode-specific "fontify" function if your mode-specific "original" buffer-local variable is nil. (This allows re-enabling your mode after it was disabled, but you weren't the outer wrapper when the mode was disabled).

This would not allow re-enabling. I do already first test whether I've wrapped, and avoid "re-wrapping" my own function.

  1. During mode disable, only replace your mode-specific "fontify" function with "original" if your "fontify" function is still what is in font-lock-fontify-region-function. If you restored "original", set your mode-specific "original" buffer-local variable to nil. (This allows only restoring if you were the outermost wrapper).

I've added this logic, see 6e08d96 and please give a test.

  1. During execution of your "fontify" function, only perform your mode-specific fontification if your mode is enabled. (This allows your "fontify" function to act as a pass-through when you weren't able to remove it during mode disable).

This isn't trivial. Much better is to come up with a way to ensure the special font-lock-fontify-region-function is removed when the mode is disabled.

  1. During execution of your "fontify" function, if your mode is disabled, check if your "fontify" function is what is in font-lock-fontify-region-function, and if it is, perform step 2. (This removes your mode as the outer-most wrapper since you couldn't remove it when your mode was previously disabled).

This will be a noop, since the function would never be called unless it's the current font-lock-fontify-region-function.

It would be preferable if font-lock-fontify-region-function were a hook of functions that could be added to/removed. But it's not. For the reasons mentioned, I do believe that wrapping the function pointed to by that variable is the right approach. As long as the inner-outer order is preserved in wrapping, then unwrapping on disable, this will always work. Only if you, e.g.

  1. enableA
  2. enableB
  3. disableA
  4. disableB

will problems occur. So I recommend seeking solutions that conserve this for A/B, and raising the issue with lsp-mode regarding applying advice to a function that may not exist for long as the font-lock-fontify-region-function. One should choose advice only as a last resort.

brownts commented 2 weeks ago

On revert the mode is not first disabled, correct?

I assume you're asking about lsp-semantic-tokens-mode. It does get disabled and re-enabled. Here is the trace for the initial buffer load in the original configuration, and then the revert. The first 4 sections cover loading, the rest is the buffer revert. You can see in the initial load how lsp-semantic-tokens-mode is enabled last, but on revert the order is changed with when indent-bars-mode is enabled respective of lsp-semantic-tokens-mode. lsp-on-revert is registered in the after-revert-hook, so I imagine that's how that's happening, but I didn't trace through it all.

======================================================================
1 -> (lsp)jinx-mod.c
1 <- lsp: #("LSP :: Connected to [clangd:1238101/starting /home/troy/.emacs.custom/elpaca/repos/jinx]." 0 3 (face success))jinx-mod.c
======================================================================
1 -> (indent-bars-mode)jinx-mod.c
1 <- indent-bars-mode: tjinx-mod.c
======================================================================
1 -> (indent-bars-mode)jinx-mod.c
1 <- indent-bars-mode: tjinx-mod.c
======================================================================
1 -> (lsp-semantic-tokens-mode 1)jinx-mod.c
1 <- lsp-semantic-tokens-mode: tjinx-mod.c
======================================================================
1 -> (lsp-semantic-tokens-mode -1)jinx-mod.c
1 <- lsp-semantic-tokens-mode: niljinx-mod.c
======================================================================
1 -> (lsp)jinx-mod.c
| 2 -> (lsp-semantic-tokens-mode 1)jinx-mod.c
| 2 <- lsp-semantic-tokens-mode: tjinx-mod.c
1 <- lsp: #("LSP :: Connected to [clangd:1238101 /home/troy/.emacs.custom/elpaca/repos/jinx]." 0 3 (face success))jinx-mod.c
======================================================================
1 -> (indent-bars-mode)jinx-mod.c
1 <- indent-bars-mode: tjinx-mod.c
======================================================================
1 -> (indent-bars-mode)jinx-mod.c
1 <- indent-bars-mode: tjinx-mod.c
brownts commented 2 weeks ago

Maybe I was unclear, but with these rules, I was suggesting how you can support the enableA, enableB, disableA, disableB ordering. It's totally possible that I'm completely missing something though.

  1. During mode enable, only set font-lock-fontify-region-function with your mode-specific "fontify" function if your mode-specific "original" buffer-local variable is nil. (This allows re-enabling your mode after it was disabled, but you weren't the outer wrapper when the mode was disabled).

This would not allow re-enabling. I do already first test whether I've wrapped, and avoid "re-wrapping" my own function.

If you're currently in the situation where your mode is disabled, but your "fontify" function is still "referenced" by an outer wrapped mode, it's not sufficient to check font-lock-fontify-region-function as it will be pointing at the outermost wrapper, and you don't want to change it because you'll create a circularity in the wrapping. That's why I had indicated that in step 2 below, you should clear indent-bars--orig-fontify-region. If you clear it on successful restoration of "orig" to font-lock-fontify-region-function, then you can just check to see if it's nil here. When it's clear, you know you're not being "referenced" by an outer wrapper.

  1. During mode disable, only replace your mode-specific "fontify" function with "original" if your "fontify" function is still what is in font-lock-fontify-region-function. If you restored "original", set your mode-specific "original" buffer-local variable to nil. (This allows only restoring if you were the outermost wrapper).

I've added this logic, see 6e08d96 and please give a test.

See comments above. I think you should clear indent-bars--orig-fontify-region when you can successfully restore font-lock-fontify-region-function so you can tell whether you're being referenced by an outer wrapper when you try to subsequently enable the mode.

  1. During execution of your "fontify" function, only perform your mode-specific fontification if your mode is enabled. (This allows your "fontify" function to act as a pass-through when you weren't able to remove it during mode disable).

This isn't trivial. Much better is to come up with a way to ensure the special font-lock-fontify-region-function is removed when the mode is disabled.

Can't you just check the indent-bars-mode variable? So in indent-bars--fontify, if indent-bars-mode is not nil, do your normal processing, otherwise just call indent-bars--orig-fontify-region. You can't always remove your "fontify" function when the mode is disabled if you're not the outer wrapper. That's the new commit you just added...to only remove when you're the outer wrapper.

  1. During execution of your "fontify" function, if your mode is disabled, check if your "fontify" function is what is in font-lock-fontify-region-function, and if it is, perform step 2. (This removes your mode as the outer-most wrapper since you couldn't remove it when your mode was previously disabled).

This will be a noop, since the function would never be called unless it's the current font-lock-fontify-region-function.

It won't be a no-op if you're not the outer wrapper when the mode was disabled. The check in your commit might get you into this situation.

It would be preferable if font-lock-fontify-region-function were a hook of functions that could be added to/removed. But it's not. For the reasons mentioned, I do believe that wrapping the function pointed to by that variable is the right approach. As long as the inner-outer order is preserved in wrapping, then unwrapping on disable, this will always work. Only if you, e.g.

  1. enableA
  2. enableB
  3. disableA
  4. disableB

will problems occur. So I recommend seeking solutions that conserve this for A/B, and raising the issue with lsp-mode regarding applying advice to a function that may not exist for long as the font-lock-fontify-region-function. One should choose advice only as a last resort.

I'm trying to provide a solution that's more flexible and allows disabling in a different order than enabling. As was hopefully described above, I don't think it's easy to keep the strict ordering that you're recommending.

If we can nail down the wrapping solution, I'm proposing that it would be applied to lsp as well. I'm just trying to come up with a solution that works for out-of-order disabling/enabling.

jdtsmith commented 2 weeks ago

I was suggesting how you can support the enableA, enableB, disableA, disableB ordering. It's totally possible that I'm completely missing something though.

Thinking further about it, there are likely too many variables to control this "automatically" for any arbitrary combination of modes which wrap or modify the fontify functions. Can you confirm that if indent-bars is loaded last, say in the lsp-semantic-tokens-mode-hook, that all works as expected?

brownts commented 2 weeks ago

Can you confirm that if indent-bars is loaded last, say in the lsp-semantic-tokens-mode-hook, that all works as expected?

For some reason, it doesn't seem to like it.

======================================================================
1 -> (lsp)jinx-mod.c
1 <- lsp: #("LSP :: Connected to [clangd:2047684/starting /home/troy/.emacs.custom/elpaca/repos/jinx]." 0 3 (face success))jinx-mod.c
======================================================================
1 -> (lsp-semantic-tokens-mode 1)jinx-mod.c
| 2 -> (indent-bars-mode)jinx-mod.c
| 2 <- indent-bars-mode: !non-local\ exit!jinx-mod.c
1 <- lsp-semantic-tokens-mode: !non-local\ exit!jinx-mod.c
Debugger entered--Lisp error: (wrong-type-argument arrayp nil)
  indent-bars-ts--update-scope1(#<buffer jinx-mod.c>)
  indent-bars-ts--custom-update-scope()
  indent-bars--custom-set(indent-bars-ts-styling-scope out-of-scope)
  custom-initialize-reset(indent-bars-ts-styling-scope (funcall #'#f(compiled-function () #<bytecode 0x18c021e769014>)))
  custom-declare-variable(indent-bars-ts-styling-scope (funcall #'#f(compiled-function () #<bytecode 0x18c021e769014>)) "Which scope the *-ts-* style variables apply to: in or out.\nBy default, the *-ts-* custom variables apply to the out-of-scope\nstyle, and in-scope bars make use of the default (non-ts)\nstyling.  If instead this is set to `in-scope', the out-of-scope\nbars share the default style, and in-scope bars are configured\nwith alternate styling using the *-ts-* variables." :type (choice (const :tag "Out of scope" out-of-scope) (const :tag "In scope" in-scope)) :set indent-bars--custom-set)
  byte-code("\300\301!\210\300\302!\210\300\303!\210\300\304!\210\300\305\306\307#\210\310\311\306\312\313\304\314\315&\7\210\310\316\306\317\313\304\314\315&\7\210\320\321\322\323\324DD\325\326\327\330\331&\7\207" [require cl-lib seq jit-lock indent-bars treesit nil t custom-declare-group indent-bars-ts "Customization group for indent-bars treesitter options." :group :prefix "indent-bars-ts-" indent-bars-ts-style "Customization group for indent-bars treesitter alternate styling." custom-declare-variable indent-bars-ts-styling-scope funcall function #f(compiled-function () #<bytecode 0x18c021e769014>) "Which scope the *-ts-* style variables apply to: in or out.\nBy default, the *-ts-* custom variables apply to the out-of-scope\nstyle, and in-scope bars make use of the default (non-ts)\nstyling.  If instead this is set to `in-scope', the out-of-scope\nbars share the default style, and in-scope bars are configured\nwith alternate styling using the *-ts-* variables." :type (choice (const :tag "Out of scope" out-of-scope) (const :tag "In scope" in-scope)) :set indent-bars--custom-set] 8)
  indent-bars--ts-mode(1)
  indent-bars-setup()
  #<subr indent-bars-mode>()
  apply(#<subr indent-bars-mode> nil)
  #f(compiled-function (body &rest args) #<bytecode -0x751c2939f88a6df>)(#<subr indent-bars-mode>)
  apply(#f(compiled-function (body &rest args) #<bytecode -0x751c2939f88a6df>) #<subr indent-bars-mode> nil)
  indent-bars-mode()
  #<subr lsp-semantic-tokens-mode>(1)
  apply(#<subr lsp-semantic-tokens-mode> 1)
  #f(compiled-function (body &rest args) #<bytecode -0x751c2411c68a6df>)(#<subr lsp-semantic-tokens-mode> 1)
  apply(#f(compiled-function (body &rest args) #<bytecode -0x751c2411c68a6df>) #<subr lsp-semantic-tokens-mode> 1)
  lsp-semantic-tokens-mode(1)
  lsp-semantic-tokens--enable()
  lsp-configure-buffer()
  lsp-managed-mode(1)
  lsp--text-document-did-open()
  lsp--open-in-workspace(#s(lsp--workspace :ewoc nil :server-capabilities #<hash-table equal 29/29 0x10f73f05dabf ...> :registered-server-capabilities nil :root "/home/troy/.emacs.custom/elpaca/repos/jinx" :client #s(lsp--client :language-id nil :add-on? nil :new-connection (:connect #f(compiled-function (filter sentinel name environment-fn workspace) #<bytecode 0x1d607b922c0000ff>) :test? #f(compiled-function () #<bytecode -0xccf22f9a8ece4cb>)) :ignore-regexps nil :ignore-messages nil :notification-handlers #<hash-table equal 0/0 0x10f73963acb2 ...> :request-handlers #<hash-table equal 0/0 0x10f73965f537 ...> :response-handlers #<hash-table eql 1/6 0x10f73965f559 ...> :prefix-function nil :uri-handlers #<hash-table equal 0/0 0x10f73965f54a ...> :action-handlers #<hash-table equal 0/0 0x10f73965f574 ...> :action-filter nil :major-modes nil :activation-fn #f(compiled-function (file-name mode) #<bytecode -0x34fd403b994a132>) :priority -1 :server-id clangd :multi-root nil :initialization-options nil :semantic-tokens-faces-overrides nil :custom-capabilities nil :library-folders-fn #f(compiled-function (workspace) #<bytecode 0x119520d107394c67>) :before-file-open-fn nil :initialized-fn nil :remote? nil :completion-in-comments? nil :path->uri-fn nil :uri->path-fn nil :environment-fn nil :after-open-fn nil :async-request-handlers #<hash-table equal 0/0 0x10f73965f5a1 ...> :download-server-fn #f(compiled-function (client callback error-callback update?) #<bytecode 0x36761b66ba7acea>) :download-in-progress? nil :buffers nil :synchronize-sections nil) :host-root nil :proc #<process clangd> :cmd-proc #<process clangd> :buffers (#<buffer jinx-mod.c>) :semantic-tokens-faces [lsp-face-semhl-variable lsp-face-semhl-variable lsp-face-semhl-parameter lsp-face-semhl-function lsp-face-semhl-method lsp-face-semhl-function lsp-face-semhl-property lsp-face-semhl-variable lsp-face-semhl-class lsp-face-semhl-interface lsp-face-semhl-enum lsp-face-semhl-constant lsp-face-semhl-type lsp-face-semhl-type nil lsp-face-semhl-namespace lsp-face-semhl-type-parameter lsp-face-semhl-interface lsp-face-semhl-type lsp-face-semhl-macro lsp-face-semhl-comment] :semantic-tokens-modifier-faces [lsp-face-semhl-interface lsp-face-semhl-deprecated nil lsp-face-semhl-constant lsp-face-semhl-static lsp-face-semhl-keyword nil nil lsp-face-semhl-default-library nil nil nil nil nil] :extra-client-capabilities nil :status initialized :metadata #<hash-table equal 0/0 0x10f73910e31f ...> :watches #<hash-table equal 0/0 0x10f73910e321 ...> :workspace-folders nil :last-id 0 :status-string nil :shutdown-action nil :diagnostics #<hash-table equal 0/0 0x10f73910e332 ...> :work-done-tokens #<hash-table equal 0/0 0x10f73910e35c ...>))
  #f(compiled-function (buffer) #<bytecode -0xbf14030e2b215a>)(#<buffer jinx-mod.c>)
  mapc(#f(compiled-function (buffer) #<bytecode -0xbf14030e2b215a>) (#<buffer jinx-mod.c>))
  #f(compiled-function (input0) #<bytecode 0x3439990e81d0e0a>)(#<hash-table equal 2/2 0x10f73f05dac2 ...>)
  #f(compiled-function (result) #<bytecode -0x14afc1fa2e0c2e4d>)(#<hash-table equal 2/2 0x10f73f05dac2 ...>)
  #f(compiled-function (result) #<bytecode -0x1083fc2129dce4d6>)(#<hash-table equal 2/2 0x10f73f05dac2 ...>)
  lsp--parser-on-message(#<hash-table equal 3/3 0x10f73f05daf3 ...> #s(lsp--workspace :ewoc nil :server-capabilities #<hash-table equal 29/29 0x10f73f05dabf ...> :registered-server-capabilities nil :root "/home/troy/.emacs.custom/elpaca/repos/jinx" :client #s(lsp--client :language-id nil :add-on? nil :new-connection (:connect #f(compiled-function (filter sentinel name environment-fn workspace) #<bytecode 0x1d607b922c0000ff>) :test? #f(compiled-function () #<bytecode -0xccf22f9a8ece4cb>)) :ignore-regexps nil :ignore-messages nil :notification-handlers #<hash-table equal 0/0 0x10f73963acb2 ...> :request-handlers #<hash-table equal 0/0 0x10f73965f537 ...> :response-handlers #<hash-table eql 1/6 0x10f73965f559 ...> :prefix-function nil :uri-handlers #<hash-table equal 0/0 0x10f73965f54a ...> :action-handlers #<hash-table equal 0/0 0x10f73965f574 ...> :action-filter nil :major-modes nil :activation-fn #f(compiled-function (file-name mode) #<bytecode -0x34fd403b994a132>) :priority -1 :server-id clangd :multi-root nil :initialization-options nil :semantic-tokens-faces-overrides nil :custom-capabilities nil :library-folders-fn #f(compiled-function (workspace) #<bytecode 0x119520d107394c67>) :before-file-open-fn nil :initialized-fn nil :remote? nil :completion-in-comments? nil :path->uri-fn nil :uri->path-fn nil :environment-fn nil :after-open-fn nil :async-request-handlers #<hash-table equal 0/0 0x10f73965f5a1 ...> :download-server-fn #f(compiled-function (client callback error-callback update?) #<bytecode 0x36761b66ba7acea>) :download-in-progress? nil :buffers nil :synchronize-sections nil) :host-root nil :proc #<process clangd> :cmd-proc #<process clangd> :buffers (#<buffer jinx-mod.c>) :semantic-tokens-faces [lsp-face-semhl-variable lsp-face-semhl-variable lsp-face-semhl-parameter lsp-face-semhl-function lsp-face-semhl-method lsp-face-semhl-function lsp-face-semhl-property lsp-face-semhl-variable lsp-face-semhl-class lsp-face-semhl-interface lsp-face-semhl-enum lsp-face-semhl-constant lsp-face-semhl-type lsp-face-semhl-type nil lsp-face-semhl-namespace lsp-face-semhl-type-parameter lsp-face-semhl-interface lsp-face-semhl-type lsp-face-semhl-macro lsp-face-semhl-comment] :semantic-tokens-modifier-faces [lsp-face-semhl-interface lsp-face-semhl-deprecated nil lsp-face-semhl-constant lsp-face-semhl-static lsp-face-semhl-keyword nil nil lsp-face-semhl-default-library nil nil nil nil nil] :extra-client-capabilities nil :status initialized :metadata #<hash-table equal 0/0 0x10f73910e31f ...> :watches #<hash-table equal 0/0 0x10f73910e321 ...> :workspace-folders nil :last-id 0 :status-string nil :shutdown-action nil :diagnostics #<hash-table equal 0/0 0x10f73910e332 ...> :work-done-tokens #<hash-table equal 0/0 0x10f73910e35c ...>))
  #f(compiled-function (msg) #<bytecode 0xba645cbf0766e22>)(#<hash-table equal 3/3 0x10f73f05daf3 ...>)
  mapc(#f(compiled-function (msg) #<bytecode 0xba645cbf0766e22>) (#<hash-table equal 3/3 0x10f73f05daf3 ...>))
  #f(compiled-function (proc input) #<bytecode -0x98eeefb304ec516>)(#<process clangd> "Content-Length: 1916\15\n\15\n{\"id\":1,\"jsonrpc\":\"2.0\",\"result\":{\"capabilities\":{\"astProvider\":true,\"callHierarchyProvider\":true,\"clangdInlayHintsProvider\":true,\"codeActionProvider\":{\"codeActionKinds\":[\"quickfix\",\"refactor\",\"info\"]},\"compilationDatabase\":{\"automaticReload\":true},\"completionProvider\":{\"resolveProvider\":false,\"triggerCharacters\":[\".\",\"<\",\">\",\":\",\"\\\"\",\"/\",\"*\"]},\"declarationProvider\":true,\"definitionProvider\":true,\"documentFormattingProvider\":true,\"documentHighlightProvider\":true,\"documentLinkProvider\":{\"resolveProvider\":false},\"documentOnTypeFormattingProvider\":{\"firstTriggerCharacter\":\"\\n\",\"moreTriggerCharacter\":[]},\"documentRangeFormattingProvider\":true,\"documentSymbolProvider\":true,\"executeCommandProvider\":{\"commands\":[\"clangd.applyFix\",\"clangd.applyTweak\"]},\"hoverProvider\":true,\"implementationProvider\":true,\"inlayHintProvider\":true,\"memoryUsageProvider\":true,\"referencesProvider\":true,\"renameProvider\":{\"prepareProvider\":true},\"selectionRangeProvider\":true,\"semanticTokensProvider\":{\"full\":{\"delta\":true},\"legend\":{\"tokenModifiers\":[\"declaration\",\"deprecated\",\"deduced\",\"readonly\",\"static\",\"abstract\",\"virtual\",\"dependentName\",\"defaultLibrary\",\"usedAsMutableReference\",\"functionScope\",\"classScope\",\"fileScope\",\"globalScope\"],\"tokenTypes\":[\"variable\",\"variable\",\"parameter\",\"function\",\"method\",\"function\",\"property\",\"variable\",\"class\",\"interface\",\"enum\",\"enumMember\",\"type\",\"type\",\"unknown\",\"namespace\",\"typeParameter\",\"concept\",\"type\",\"macro\",\"comment\"]},\"range\":false},\"signatureHelpProvider\":{\"triggerCharacters\":[\"(\",\")\",\"{\",\"}\",\"<\",\">\",\",\"]},\"standardTypeHierarchyProvider\":true,\"textDocumentSync\":{\"change\":2,\"openClose\":true,\"save\":true},\"typeDefinitionProvider\":true,\"typeHierarchyProvider\":true,\"workspaceSymbolProvider\":true},\"serverInfo\":{\"name\":\"clangd\",\"version\":\"clangd version 15.0.6 (https://github.com/llvm/llvm-project 088f33605d8a61ff519c580a71b1dd57d16a03f8) linux+grpc x86_64-unknown-linux-gnu\"}}}")
jdtsmith commented 2 weeks ago

Remove all the hook stuff and try just enabling by hand M-x indent-bars-mode. If the error persists, compile the last named function (here indent-bars-ts--update-scope1) by navigating to it and using C-M-x, then send the more detailed traceback.

brownts commented 2 weeks ago

Remove all the hook stuff and try just enabling by hand M-x indent-bars-mode. If the error persists, compile the last named function (here indent-bars-ts--update-scope1) by navigating to it and using C-M-x, then send the more detailed traceback.

It's seems to happen the very first time indent-bars-mode is enabled in this particular scenario, whether enabled manually or as part of the hook. I did perform C-M-x, on indent-bars-ts--update-scope1 but it didn't provide any more detail in the traceback. Maybe I need to do something else?

Here is the traceback from enabling it manually:

Debugger entered--Lisp error: (wrong-type-argument arrayp nil)
  indent-bars-ts--update-scope1(#<buffer jinx-mod.c>)
  indent-bars-ts--custom-update-scope()
  run-hooks(indent-bars-custom-set)
  apply(run-hooks indent-bars-custom-set)
  indent-bars--custom-set(indent-bars-ts-styling-scope out-of-scope)
  custom-initialize-reset(indent-bars-ts-styling-scope (funcall #'#f(compiled-function () #<bytecode 0x18c01e3ca1b14>)))
  custom-declare-variable(indent-bars-ts-styling-scope (funcall #'#f(compiled-function () #<bytecode 0x18c01e3ca1b14>)) "Which scope the *-ts-* style variables apply to: in or out.\nBy default, the *-ts-* custom variables apply to the out-of-scope\nstyle, and in-scope bars make use of the default (non-ts)\nstyling.  If instead this is set to `in-scope', the out-of-scope\nbars share the default style, and in-scope bars are configured\nwith alternate styling using the *-ts-* variables." :type (choice (const :tag "Out of scope" out-of-scope) (const :tag "In scope" in-scope)) :set indent-bars--custom-set)
  byte-code("\300\301!\210\300\302!\210\300\303!\210\300\304!\210\300\305\306\307#\210\310\311\306\312\313\304\314\315&\7\210\310\316\306\317\313\304\314\315&\7\210\320\321\322\323\324DD\325\326\327\330\331&\7\207" [require cl-lib seq jit-lock indent-bars treesit nil t custom-declare-group indent-bars-ts "Customization group for indent-bars treesitter options." :group :prefix "indent-bars-ts-" indent-bars-ts-style "Customization group for indent-bars treesitter alternate styling." custom-declare-variable indent-bars-ts-styling-scope funcall function #f(compiled-function () #<bytecode 0x18c01e3ca1b14>) "Which scope the *-ts-* style variables apply to: in or out.\nBy default, the *-ts-* custom variables apply to the out-of-scope\nstyle, and in-scope bars make use of the default (non-ts)\nstyling.  If instead this is set to `in-scope', the out-of-scope\nbars share the default style, and in-scope bars are configured\nwith alternate styling using the *-ts-* variables." :type (choice (const :tag "Out of scope" out-of-scope) (const :tag "In scope" in-scope)) :set indent-bars--custom-set] 8)
  indent-bars--ts-mode(1)
  indent-bars-setup()
  indent-bars-mode(toggle)
  funcall-interactively(indent-bars-mode toggle)
  command-execute(indent-bars-mode record)
  execute-extended-command(nil "indent-bars-mode" #("indent-bars-mode" 0 16 (ws-butler-chg chg)))
  funcall-interactively(execute-extended-command nil "indent-bars-mode" #("indent-bars-mode" 0 16 (ws-butler-chg chg)))
  command-execute(execute-extended-command)
jdtsmith commented 2 weeks ago

What is the value of indent-bars-ts-current-scope in the jinx buffer right after this error is thrown? I don't recognize the compiled function there. Something is wrong with this traceback, so you must have misconfigured things somehow. Are you doing something in indent-bars--ts-mode-hook to try to set custom variables?

brownts commented 2 weeks ago

What is the value of indent-bars-ts-current-scope in the jinx buffer right after this error is thrown? I don't recognize the compiled function there.

I believe I've figured out what is going on here. This is unrelated to the original issue, but I don't think it was noticed until I started tracing the functions and toggled "debug on error".

The value of indent-bars-ts-current-scope is nil. It appears the only place this is initialized is within indent-bars-ts--setup, but only if there are scope specifiers in indent-bars-treesit-scope for the current language. That seems to be the problem. I didn't have any scopes for "C" in there. Once I added one, then this problem stopped.

Something is wrong with this traceback, so you must have misconfigured things somehow. Are you doing something in indent-bars--ts-mode-hook to try to set custom variables?

No, I'm not doing anything like that. I think it might just be the normal loading of indent-bars-ts.el caused by calling the autoloaded indent-bars--ts-mode from indent-bars-setup.

jdtsmith commented 2 weeks ago

Hmm, if there is no configured scope for the language in the buffer, there is indeed no call to indent-bars-ts--setup, but there is also no call to any of the scope update functions; it would be as if you hadn't configured tree-sitter for use at all.

Perhaps you are calling setopt or similar in the mode hook? Otherwise, indent-bars--ts-mode should not be calling all that custom-set stuff in your backtrace, which is the actual problem.

In any case, I've just added a guard to prevent scenarios like this. Could you please try it out (removing again your "C" scope setting to see if it no longer hits an error)?

brownts commented 2 weeks ago

Perhaps you are calling setopt or similar in the mode hook? Otherwise, indent-bars--ts-mode should not be calling all that custom-set stuff in your backtrace, which is the actual problem.

I'm doing nothing like that. Both indent-bars-mode-hook and indent-bars--ts-mode-hook are nil. I am 99% sure what you see is the loading of indent-bars-ts.el happening here. As I mentioned, this only occurs the first time. I defer as much package loading as possible, therefore when indent-bars-setup calls indent-bars--ts-mode, it ends up calling the autloaded version of indent-bars--ts-mode, not the "real" one (which hasn't been invoked yet). The autoloaded version causes indent-bars-ts.el to be loaded and it starts evaluating the file, starting with the first defcustom (i.e., indent-bars-ts-styling-scope). Thus I believe what you see on the backtrace from indent-bars--ts-mode to the top is the evaluation of the first defcustom macro in indent-bars-ts.el. Because you specified a setter as part of the defcustom, it's being invoked to "set" the option. If you don't want this behavior you should add an :initialize parameter set to custom-initialize-default, or maybe use a variable watcher instead.

jdtsmith commented 2 weeks ago

That's a good idea to try, thanks. I've just pushed a commit which adds :initialize across the board. With that and the guard from the prior commit, you should never see this error, but please check and let me know if so.

brownts commented 2 weeks ago

This now works as expected, thanks!

shipmints commented 1 week ago

FYI, just this morning, I was experiencing an almost identical issue, though on a python buffer and repeatable in fresh Emacs sessions. My indent-bars config uses :demand t and :init to set all variables in advance of indent-bars mode being invoked. The latest master solves this for me, too.

jdtsmith commented 1 week ago

Release made.