Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add missing default highlight groups #219

Closed
wants to merge 1 commit into from

Conversation

mehalter
Copy link
Contributor

@mehalter mehalter commented Feb 7, 2023

I noticed that there are some missing default highlight groups. I ran into this while using require("aerial").get_location(true) to build a breadcrumbs section in the statusline and some highlight groups were missing.

Thanks for all your hard work on this plugin! It's truly a gem!

@stevearc
Copy link
Owner

Thanks for the PR! Happy to merge it, but I'd like to dig in a bit first so I understand what's going on. The current LSP spec doesn't list any of those symbol kinds, and neither does the upcoming spec. What language servers are you using that returns these values, and can you give me a short code snippet that includes one or two of them?

@mehalter
Copy link
Contributor Author

Hey @stevearc ! I'll be honest, I'm not completely sure. I am the lead developer of AstroNvim and someone that uses it ran into this issue where a Snippet type of symbol was being returned by Aerial when using the get_location function (we use it to build our winbar) and was causing issues because there isn't a default highlight defined. I am just trying to help resolve that issue. I got the names for these missing pieces from the Aerial source code to see what symbols it technically supports from what is shown in the configuration: https://github.com/stevearc/aerial.nvim/blob/master/lua/aerial/config.lua#L275-L335

I can see if I can figure out what language servers it was that was returning these types of symbols but it might take me a few days.

Thanks for all your help in advance!

@stevearc
Copy link
Owner

Ah, those icons were copied from lspkind.nvim and I didn't think much about it at the time, but there's several values that aren't a SymbolKind. After looking around, it seems that they are part of a different enum value, CompletionItemKind. This makes sense, because lspkind.nvim aims to provide symbols inside the completion popup. It still doesn't explain why a Snippet type would be returned from get_location, though. None of the LSP APIs we consume should be returning that value.

Could you point me to the user report? I'm happy to take this on from the AstroNvim side, and I really would like to figure out what's going on here.

@mehalter
Copy link
Contributor Author

Yeah I can let you know if I can get the user to be able to replicate the issue. They say they have run into this a lot but cannot currently replicate. I also couldn't think of a reason why it would be happening. For now I'm just going to close this PR. If I'm able to replicate or get you some details where Aerials get_location returns some weird results I'll definitely comment back here and let you know! Thanks in advanced for giving this a think and offering to take a look at things! Sorry for the potentially unnecessary noise

@mehalter mehalter closed this Feb 13, 2023
@mehalter
Copy link
Contributor Author

Ok @stevearc ! I found an example for you reported by an AstroNvim user! With this file test.tsx with tsserver attached:

import { GAME_TITLE } from "../../constants/strings";
import { Hamburger, Question } from "../../icons";
import { ReactComponent as TimelineLogo } from "../../icons/Timeline.svg";
import { Button } from "../button/Button";

type Props = {
  setIsInfoModalOpen: (value: boolean) => void;
  setIsStatsModalOpen: (value: boolean) => void;
  setIsDatePickerModalOpen: (value: boolean) => void;
  setIsSettingsModalOpen: (value: boolean) => void;
};

export const Navbar = ({
  setIsDatePickerModalOpen,
  setIsInfoModalOpen,
}: Props) => {
  return (
    <div className="flex items-center justify-between p-3" >
      <Button
        variant="secondary"
        isIconButton
        onClick={() => setIsDatePickerModalOpen(true)}
      >
        <Hamburger />
        < /Button>

        < h1 className="flex flex-col items-center gap-2 text-center text-base leading-none dark:text-white" >
          {GAME_TITLE} < span className="sr-only" > by Timeline < /span>
            < span className="flex items-end gap-1 text-2xs leading-none text-blackAlpha-700 dark:text-whiteAlpha-700" >
              by < TimelineLogo width={71} height={10} />
            </span>
            < /h1>

            < Button
              isIconButton
              variant="secondary"
              onClick={() => setIsInfoModalOpen(true)}
            >
              <Question />
              < /Button>
              < /div>
              );
};

If you go to a line in the type Props = { ... } section, say the setIsInfoModalOpen: (value: boolean) => void; line I am getting the following result from require("aerial").get_location()

{ {                                                                                                                               
    col = 0,                                                                                                                      
    icon = " ",                                                                                                                   
    kind = "Type",                                                                                                                
    lnum = 6,                                                                                                                     
    name = "Props"                                                                                                                
  } }   

This is not a real kind in the LSP spec and is causing code to fail that is trying to get the built in Aerial highlight for the reported kind. I should add some fail safes for this on the AstroNvim side anyway, but it does appear as some sort of error. I also cannot get this to happen every single time, it seems to be extremely consistent when I open the file in an already created session say with :e or fuzzy finder.

Here is a terminal recording showing me replicating the issue: https://asciinema.org/a/q9eAYNw2HVymiNJwH4h0sr7RC

I'm not sure how the internals of Aerial work or why that would be popping up where the kind is not part of the LSP specification similar to how a previous user got a Snippet kind. Any insight would be super great and thanks so much in advanced to looking into this!

@mehalter
Copy link
Contributor Author

Actually @stevearc I think this might be coming from the treesitter backend. The error goes away once the language server initializes so it's definitely coming from the treesitter provided backend that we have active before the LSP is setup

@mehalter
Copy link
Contributor Author

Yes, I can confirm that I can replicate this 100% of the time if I disable the lsp backend and just us treesitter

@mehalter
Copy link
Contributor Author

I think what we want to do is to make sure that we have symbols/icons and highlights for each treesitter node type that is here: https://github.com/nvim-treesitter/nvim-treesitter/blob/master/CONTRIBUTING.md#highlights

I have been looking for a better resource for a full list of documented node types for nvim-treesitter but it's been a bit hard. I think the highlights is the best place

@stevearc
Copy link
Owner

Ah shit, this makes sense now. Thank you for coming up with a repro! I have made some mistakes, and I've also allowed some contributions that have had mistakes in the treesitter backend. I'll tighten these up and see if it fixes your issue.

@mehalter
Copy link
Contributor Author

Thanks so much! Let me know if you want me to do any testing for you as you make changes. Thanks so much for this plugin as well, the support for different backends is incredibly useful for supporting basically every buffer I ever have open :)

stevearc added a commit that referenced this pull request Feb 14, 2023
The icon list had several values that are not SymbolKind, they were
CompletionItemKind (which we don't need). I've removed those values, and
also added nerd font icons for SymbolKinds that didn't have them before.
@stevearc
Copy link
Owner

@mehalter okay, I believe I have fixed the issue and also added checks to make sure it doesn't happen again. Let me know if you still see any problems!

@mehalter
Copy link
Contributor Author

Awesome, seems to be working perfectly in my quick testing. The AstroNvim community is forever in your debt!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants