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

feat: add nvim-navic support #87

Merged
merged 1 commit into from
Oct 25, 2022
Merged

feat: add nvim-navic support #87

merged 1 commit into from
Oct 25, 2022

Conversation

dsully
Copy link
Contributor

@dsully dsully commented Oct 24, 2022

No description provided.

@andersevenrud
Copy link
Owner

I'm not familiar with navic. Could you provide a before and after screenshot of this ? 😊

@dsully
Copy link
Contributor Author

dsully commented Oct 24, 2022

Sure - it goes in your status line. Before:

CleanShot 2022-10-24 at 12 03 28@2x

After:

CleanShot 2022-10-24 at 12 01 56@2x

@andersevenrud
Copy link
Owner

Ah, so it's just plain by standard. I skimmed the codebase and assumed it inherited from standard hl groups.

Speaking of... how would this look if you took the same definitions as given here https://github.com/andersevenrud/nordic.nvim/blob/main/lua/nordic/colors/syntax.lua ?

@dsully
Copy link
Contributor Author

dsully commented Oct 24, 2022

Search for "For highlights to work," in https://github.com/SmiteshP/nvim-navic/blob/master/README.md (it's hidden under a fold).

Not sure I understand your question. Are you proposing adding the Navic* highlight names to the various sections in that file?

@andersevenrud
Copy link
Owner

andersevenrud commented Oct 24, 2022

Not sure I understand your question

I was thinking of if you took the definitions from that file, i.e. "variable" is "dark_white" (compared to "blue" in this PR), "numbers" is "purle" there, and so on.

Edit: I'm totally open for suggestions here though. I just wanna see how it looks if the hl's were re-used.

Edit again: ... and also if this would look like the "before" version. Because then we could settle on some new colors. Because I think aerial.nvim has some of the same stuff :)

@dsully
Copy link
Contributor Author

dsully commented Oct 24, 2022

CleanShot 2022-10-24 at 16 31 34@2x

With as good a mapping as I could get - there are things like NavicIconsFile that don't have an equivalent in syntax.lua.

This is what the colors would look like in code:

return function(c, s, cs)
    return {
        { 'NavicIconsFile', c.blue, c.gray },
        { 'NavicIconsModule', c.blue, c.gray },
        { 'NavicIconsNamespace', c.dark_white, c.gray },
        { 'NavicIconsPackage', c.cyan, c.gray },
        { 'NavicIconsClass', c.cyan, c.gray },
        { 'NavicIconsMethod', c.dark_white, c.gray },
        { 'NavicIconsProperty', c.blue, c.gray },
        { 'NavicIconsField', c.blue, c.gray },
        { 'NavicIconsConstructor', c.dark_white, c.gray },
        { 'NavicIconsEnum', c.yellow, c.gray },
        { 'NavicIconsInterface', c.yellow, c.gray },
        { 'NavicIconsFunction', c.dark_white, c.gray },
        { 'NavicIconsVariable', c.dark_white, c.gray },
        { 'NavicIconsConstant', c.dark_white, c.gray },
        { 'NavicIconsString', c.green, c.gray },
        { 'NavicIconsNumber', c.purple, c.gray },
        { 'NavicIconsBoolean', c.purple, c.gray },
        { 'NavicIconsArray', c.cyan, c.gray },
        { 'NavicIconsObject', c.cyan, c.gray },
        { 'NavicIconsKey', c.blue, c.gray },
        { 'NavicIconsNull', c.dark_white, c.gray },
        { 'NavicIconsEnumMember', c.cyan, c.gray },
        { 'NavicIconsStruct', c.cyan, c.gray },
        { 'NavicIconsEvent', c.purple, c.gray },
        { 'NavicIconsOperator', c.blue, c.gray },
        { 'NavicIconsTypeParameter', c.dark_white, c.gray },
        { 'NavicText', c.dark_white, c.gray },
        { 'NavicSeparator', c.cyan, c.gray },
    }
end

A lot more dark_white and generally monochromatic compared to the code in the PR right now. I personally don't prefer this look.

@andersevenrud andersevenrud merged commit d5456d9 into andersevenrud:main Oct 25, 2022
@andersevenrud
Copy link
Owner

I think your definitions looked better as well.

Could you maybe look at the aerial definitions to see if there's a need to synchronize the changes ?

@dsully dsully deleted the navic branch October 25, 2022 14:38
@dsully
Copy link
Contributor Author

dsully commented Oct 25, 2022

I don't use aerial myself. But it has a lot fewer definitions. I assume you'd want to use the navic colors in place of what is there where they align?

@andersevenrud
Copy link
Owner

andersevenrud commented Oct 25, 2022 via email

@dsully
Copy link
Contributor Author

dsully commented Oct 25, 2022

Done in #88

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.

2 participants