Attach parser to qf buffer to let text highlight at real time

[xzbdmw]

This is a trick I learned from trouble, however this fits very good with quicker.nvim, because when you edit the buffer, the highlighter get called on every redraw just as a normal file.

iShot_2025-02-02_14.52.28.mp4

[xzbdmw]

Instead of callingvim.treesitter.languagetree.new in trouble's code, we need to call get_parser, which will register redraw callback for current buffer (trouble manually calls attach every time item chanegs, so it does not need to call get_parser).

[xzbdmw]

The highlighter needs to own the entire buffer, because its highlight state is ephemeral, any other non-ephemeral highlight group will mess them up (they shift with text).

[stevearc]

Is it any worse than the current highlighting behavior? It already doesn't look very good when editing the text.

[xzbdmw]

Do you mean we should query buffer highlights when possible?

[xzbdmw]

There are some strange issue with EM_QUAD, I'm not sure the exact cause, grep for "cmp.mapping" result in something like this (I've only found this case the highlighter did not work well):

If you put this line into an empty lua file they look the same (except for lsp semantic highlight)
� ["<d-v>"] = cmp.mapping(function(fallback)

This <83> is something from EM_QUAD, adding a space after the EM_QUAD solves this problem. The edge case of choosing the correct range start to be included in parser turned out to be tricky, and I find this is the simplest way to avoid all sorts of problems, that is, includes an extra space before the real code, for example, "␣" stands for the extra space：

README.md ␣┃10┃text
          |        | -- we include the space to parse

This allows README.md ␣┃10┃"some string“ text to be highlighted correctly when " is at start of the qf text. However this affects all the existing screen tests (I can adjust the tests if you think this is the right way).

[stevearc]

One of the strengths of the current approach is that it handles large lists very well. For a long running asynchronous grep process, for example, as we append new items we only have to parse and highlight those new items. It looks like this method would have to reparse and rehighlight the entire quickfix list. Is there any way around that?

These issues you're having with the EM_QUAD are likely because it's a multibyte unicode character. You probably have an off-by-one error somewhere that's causing it to get split, creating an invalid utf-8 byte sequence. This should be solvable without adding an extra space.

[stevearc]

What problem is this solving? I don't rely on QuickFixCmdPost in the rest of this plugin because some methods of setting/updating the quickfix do not trigger it.

[stevearc]

We'll need some way to clear this cache when the buffer is deleted, right?

[xzbdmw]

I think QuickFixCmdPost can be changed to BufWipeOut.

[stevearc]

If we ship this feature, I think that ideally it would replace the current treesitter methods of highlighting; both the "fast" and the version with loaded buffers. If it's noticeably worse in some situations, then we can consider some way to make the other mechanisms work with it.

[xzbdmw]

This do has a problem: when list contains more than 10k items, each keypress adds like 30ms input latency.

[stevearc]

Seems like it would be possible to add an option to do a single parse pass and then detach if the buffer has > N lines, right? And better than maintaining two different ways to do highlighting.

[xzbdmw]

I've removed the old string_parser code and add a new config option highlight.max_lines

[stevearc]

Is it any worse than the current highlighting behavior? It already doesn't look very good when editing the text.

[xzbdmw]

For a long running asynchronous grep process, for example, as we append new items we only have to parse and highlight those new items. It looks like this method would have to reparse and rehighlight the entire quickfix list. Is there any way around that?

We can stop parsing lines with the same content, somehow cache it, however parsing is not the bottleneck here, it is vim.filetype.match the slowest, luckily 0.11 will speeds it up from 0.5ms to 0.05ms each item.

You probably have an off-by-one error somewhere that's causing it to get split

Actually I tried all the combination of offset and it will only highlight local but not local if you insert at the very start if the line without an additional space.

[xzbdmw]

I’m not sure how useful cache regions are, as parsing is generally very fast. With 0.11 introducing asynchronous parsing, testing in the nightly build shows that parsing 7,000 items takes 144ms, while the quickfixTextFunc itself requires 200ms.

[stevearc]

I tested it locally and made some changes to the EM_QUAD logic. I tested it against your "grep cmp.mapping" and it seems to work fine.

[stevearc]

What is empty_regions for?

[xzbdmw]

empty_regions is used to to clear the included regions and make sure a reparse with clean state will happen after Refresh, if you modify the saved file and call Refresh on qf the previous highlight are still there:

iShot_2025-02-16_09.58.15.mp4

[xzbdmw]

Edit: this doesn't work, we have to manually include an empty region, multiple call to vim.treesitter.get_parser with the same argument returns the same parser.

If we want to support max_lines option, then empty_region is not needed, because lines are computed using regions, it complicates the logic of when should we register the callback (vim.treesitter.get_parser) and when should not (vim.treesitter.languagetree.new), now I'm just creating a new parser each time attach is called.

[xzbdmw]

iShot_2025-02-16_09.42.27.mp4

the original highlight is correct but it has problem reparsing (This only happens if I insert at line start)

[xzbdmw]

      if loaded then
        -- If this is called together with  the parser, they indeed will overwrite each in some cases
        add_item_highlights_from_buf(qfbufnr, item, line, i)
      end

These highlights comes from semantic highlight, two problems, one is everything after // should be gray and second is the semantic highlight left behind after saving.
https://github.com/user-attachments/assets/97b6a831-2651-464c-826d-7225380d860c

[xzbdmw]

I think there remains two one question

1. should we include semantic highlight with parser? I think this is just the config option lsp left user to decide
2. is the space after EM_QUAD unavoidable?

[xzbdmw]

Sigh, owo more problem, you must perform an edit, if not, directly delete a line loses all the highlights:

iShot_2025-02-16_15.27.37.mp4

we can do something like this as workaround:

  -- Run a full parse for all included regions. There are two reasons:
  -- 1. When we call `vim.treesitter.get_parser`, we have not set any
  --    injection ranges.
  -- 2. If this is not called, the highlighter will do incremental parsing,
  --    which means it only parses visible areas (the on_win and on_line callback),
  --    so if we modify the buffer, unvisited area's state get unsynced.
  pcall(parser.parse, parser, true)
  vim.api.nvim_buf_call(buf, function()
    vim.cmd([[norm! "_xu]])
  end)

though it is a bit too hacky.

[stevearc]

Sorry for the long wait; I don't have as much free time as I used to. Loading the context back into my head, these are the remaining questions:

1. What to do with the space after the EM_QUAD?
2. Highlights disappear after deleting a line
3. Is there anything pending in this thread?

For 1 I would say that I strongly bias towards not adding a space after the EM_QUAD. If the only negative impact is that inserting at the very beginning of a line doesn't get highlighted, I think that's still better than adding a bonus space everywhere. I still think that this should be able to work completely fine without the space, but it's possible that there's a fix or adjustment that would have to be made upstream.

For 2, that is very weird. I definitely don't like the hack approach. I don't think I fully understand what is going wrong here. I know that the treesitter highlights attempt to only update the regions that are visible, but why does that cause not-currently-visible regions to get desynced? Is there a bug in the incremental logic, or is this because of how we're setting/using the regions?

[xzbdmw]

Is there a bug in the incremental logic, or is this because of how we're setting/using the regions?

My guess is that if you don't perform an edit, the entire injection tree get invalidated when one entire line disappears, this must have something to do with the LanguageTree internals's assumption.

If the only negative impact is that inserting at the very beginning of a line doesn't get highlighted, I think that's still better than adding a bonus space everywhere

This is not ideal when you add a comment leader at the very beginning of the line though, user will expect entire line get dimmed, since you can't use gcc in qf buffer, mostly one will use I// instead.

Maybe we can use extmarks to conceal the extra space?

Is there anything pending #41 (comment)

This is addressed.

That said, I'm not in a hurry to merge this PR, and there are many treesitter changes in the comming 0.11 stable version, we can wait for it.

[xzbdmw]

Just push a commit conceals the space, seems works well.

Edit: for conceal to take effect in insert mode we have to set concealcursor, and that confilct if the qf text is concealed, e.g. markdown files, This approach is still not ideal.

Another option is to remove EM entirely, replace it with a space, and not rely on EM to calculate necessary width information, not sure how much complexity will be added by this approach.

[stevearc]

Okay, in that case let's wait for 0.11 and see if it provides fixes for any of the issues