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 syntax highlight inconsistencies in Go code #246

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

hoenirvili
Copy link

@hoenirvili hoenirvili commented Feb 8, 2021

This fixes #245 .

Function calls and format specifier are not printed correctly.
Usually people are using `faith/vim-go` to group these highlights.
We add the corresponding color the vim-go groups. Still there
are some groups that are not present/defined in vim-go. I've added
a comment there for future fix.

Signed-off-by: hoenirvili <[email protected]>
arcticicestudio and others added 12 commits June 9, 2021 21:47
Neovim version 0.5 [1] is a long-time awaited update that introduces
features like tree-sitter [2][3] support and native LSP [4][5].
Even though Neovim devides more and more from Vim through specific
features like first-class Lua support with custom APIs, the highlighting
for tree-sitter is achived through "normal" syntax highlighting groups.
Most of the groups are already linked by the nvim-treesitter plugin by
default [6] so only a few groups have been adjuated for now to fit the
Nord style.

[1]: https://github.com/neovim/neovim/releases/tag/v0.5.0
[2]: https://github.com/tree-sitter/tree-sitter
[3]: https://github.com/nvim-treesitter/nvim-treesitter
[4]: https://neovim.io/doc/user/lsp.html
[5]: https://github.com/neovim/nvim-lspconfig
[6]: https://github.com/nvim-treesitter/nvim-treesitter/blob/90f15d9/plugin/nvim-treesitter.vim

Co-authored-by: Ferran Jovell <[email protected]>
Co-authored-by: Arctic Ice Studio <[email protected]>
Co-authored-by: Sven Greb <[email protected]>

Closes nordthemeGH-235
Add support for `vim-pandoc/vim-pandoc-syntax`

To improve syntax highlighting for Pandoc [1], support for the
vim-pandoc/vim-pandoc-syntax [2] plugin has been implemented.
Most groups are linked to existing Markdown groups to ensure a
consistent style across languages and different plugins.

Configurations used for testing:

```vim
let g:pandoc#syntax#conceal#cchar_overrides = {"atx": "〉"}
let g:nord_italic = 1
```

Resources:

- `g:pandoc#syntax#conceal#cchar_overrides` docs [3]
- `s:cchars` definition [4]
- Markdown "Extended Syntax" Guide [5]

[1]: https://pandoc.org
[2]: https://github.com/vim-pandoc/vim-pandoc-syntax
[3]: https://github.com/vim-pandoc/vim-pandoc-syntax/blob/68d7249/doc/pandoc-syntax.txt#L45-L50
[4]: https://github.com/vim-pandoc/vim-pandoc-syntax/blob/5056e63/syntax/pandoc.vim#L37-L72
[5]: https://www.markdownguide.org/extended-syntax

Co-authored-by: Arctic Ice Studio <[email protected]>
Co-authored-by: Sven Greb <[email protected]>

Closes nordthemeGH-220
Support for LSP code lenses

Before LSP code lenses [1] where code lenses were highlighted with the
default color which has been changed to make it less visually intrusive,
like other UI related elements, i.e. messages of linters.

[1]: https://neovim.io/doc/user/lsp.html#lsp-highlight-codelens

nordthemeGH-266


Co-authored-by: Arctic Ice Studio <[email protected]>
The `TSError` group is used to highlight syntax/parser errors [1] which
caused an aggressive styling where the background color of many syntax
elements was rendered with `nord11` during typing. This was caused due
to the fast processing of `tree-sitter` which also resulted in highlight
flickering.
This is a known problem and was fixed by many other themes (e.g. Dracula
[2]) by removing the group again. One of the core maintainers of
`nvim-treesitter` provided a solution by remapping groups [3] and also
mentioned that the group is styled by the `nvim-treesitter` plugin but
the active theme [4].

Syntax errors can still be highlighted through linters and parsers like
Neovim's LSP [5] can still be used instead to highlight errors with the
correct style (e.g. only change the foreground color of a single word).

[1]: https://github.com/nvim-treesitter/nvim-treesitter/blob/fb5d6e04/doc/nvim-treesitter.txt#L493-L495
[2]: dracula/vim#232
[3]: nvim-treesitter/nvim-treesitter#78 (comment)
[4]: nvim-treesitter/nvim-treesitter#1016 (comment)
[5]: https://github.com/neovim/nvim-lspconfig

Fixes nordthemeGH-269
…me#282)

In Neovim `0.6.0` [1] the naming scheme for the highlight groups of the
diagnostic API changed [2]. The new groups have been added as default while
the previous groups are conditionally guarded when using Neovim `0.5.0`.

[1]: https://github.com/neovim/neovim/releases/tag/v0.6.0
[2]: neovim/neovim@a5bbb93#diff-51fab2b766d0a3b606462e95de492190df173b7296147912307cdad636cd492aR77


Co-authored-by: Arctic Ice Studio <[email protected]>
Co-authored-by: Sven Greb <[email protected]>

nordthemeGH-282
Support for Neovim LSP `documentHighlight` groups

The Neovim LSP `textDocument` / `documentHightlight` groups [1] are
responsible to highlight tokens in a document that are related to each
other, e.g. decalred variables, using the
`vim.buf.lsp.document_highlight()` function.
Also see the LSP specification about "Document Highlights Request" [2]
for more details.

[1]: https://github.com/neovim/neovim/blob/f92a2457c2e7ad14d9a5a907ef4213fa770b6d95/runtime/doc/lsp.txt#L423
[2]: https://microsoft.github.io/language-server-protocol/specification#textDocument_documentHighlight


Co-authored-by: Arctic Ice Studio <[email protected]>
Co-authored-by: Sven Greb <[email protected]>
Improve readability of C language constants

To improve the readability of C language constants, defined by the
`cConstant` syntax highlighting group [1], these are now colored with
`nord9` for the foreground to make them stand out. This is important in
C, since interesting things are usually happening in their proximity,
like checking/returning an error, passing particular values/flags to
functions and so on.

[1]: https://github.com/vim/vim/blob/0e6adf8a29d5c2c96c42cc7157f71bf22c2ad471/runtime/syntax/c.vim#L313-L375



Co-authored-by: Arctic Ice Studio <[email protected]>
Co-authored-by: Sven Greb <[email protected]>

nordthemeGH-283
cmoscofian and others added 2 commits February 19, 2022 14:22
The `vim.lsp.buf.signature_help` function is used to highlight the
active parameter in the signature help [1].
Before this commit the active parameter was not styled differently to
any other parameter which made it hard to distinguish it. This has been
improved by adding support for the `LspSignatureActiveParameter` syntax
highlighting group where the active parameter now uses `nord8` are
foreground color and additionally a font underline with the same color.

[1]: https://github.com/neovim/neovim/blob/70db972e5fbcab39946ad8ac05472a693cf65b68/runtime/doc/lsp.txt#L456-L459


Co-authored-by: Sven Greb <[email protected]>
Co-authored-by: Arctic Ice Studio <[email protected]>

nordthemeGH-286
colors/nord.vim Outdated
@@ -591,6 +592,30 @@ call s:hi("CocErrorSign" , s:nord11_gui, "", s:nord11_term, "", "", "")
call s:hi("CocInfoSign" , s:nord8_gui, "", s:nord8_term, "", "", "")
call s:hi("CocHintSign" , s:nord10_gui, "", s:nord10_term, "", "", "")

" vim-go
" > faith/vim-go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fatih, not faith

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@hoenirvili hoenirvili force-pushed the fix-go-syntax-highlight branch from 9e6115d to 1af90ba Compare April 4, 2022 06:34
Signed-off-by: hoenirvili <[email protected]>
@hoenirvili hoenirvili force-pushed the fix-go-syntax-highlight branch from 1af90ba to 09aff38 Compare April 4, 2022 06:35
hi! link goEscapeU FormatSpecifier
hi! link goEscapeBigU FormatSpecifier
hi! link goEscapeError FormatSpecifier
" TODO: these are not exposed by vim-go so you need to add
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please open an issue and/or a pull-request at https://github.com/fatih/vim-go for exposing those :)

@@ -591,6 +592,30 @@ call s:hi("CocErrorSign" , s:nord11_gui, "", s:nord11_term, "", "", "")
call s:hi("CocInfoSign" , s:nord8_gui, "", s:nord8_term, "", "", "")
call s:hi("CocHintSign" , s:nord10_gui, "", s:nord10_term, "", "", "")

" vim-go
" > fatih/vim-go
call s:hi("goBuiltins", s:nord7_gui, "", s:nord7_term, "", "", "")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please wrap this block between a if exists("g:go_loaded_install") … endif, like it's done in #294

@svengreb
Copy link
Member

svengreb commented Apr 5, 2022

Hi @hoenirvili 👋, thanks for your contribution 👍
I'm currently busy with the preparations for the roadmap and future plan of the whole Nord project, but I'll try to review the changes within the next week(s).

@svengreb svengreb changed the base branch from develop to master May 14, 2022 12:49
@svengreb svengreb removed the request for review from arcticicestudio June 4, 2023 09:54
@svengreb svengreb assigned svengreb and unassigned hoenirvili Jun 4, 2023
@svengreb
Copy link
Member

svengreb commented Jun 4, 2023

Thank you for your patience! 🙏🏼
It‘s been a while since I had free time to focus more on Nord, and my open source projects in general, and invest time in this issue due to work-life balance.

I recently published the first “Northern Post — The state and roadmap of Nord“ announcement which includes all details about the plans and future of the Nord project, including the goal of catching up with the backlog. This issue is part of the backlog and therefore I want to triage and process it to get one step closer to a “clean state“. Read the announcement about reaching the “clean“ contribution triage state in Nord‘s discussions for more details about the goal.

Therefore it has been added to the queue in the central and single-source-of-truth project board that is also described in more detail in the roadmap announcement.


@hoenirvili Thanks again for your contribution 🚀
The review will be planned for the next iterations!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: queued
Development

Successfully merging this pull request may close these issues.

My colorscheme is incorrect on Go syntax.
8 participants