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 highlighting for function return types when highlight_function_arguments is enabled #2116

Merged
merged 1 commit into from
Jan 5, 2019

Conversation

pocke
Copy link
Contributor

@pocke pocke commented Jan 5, 2019

Problem

vim-go does not highlight function return types with brackets correctly when g:go_highlight_function_arguments = 1 is setted.

expected highlight

190106014753

actual highlight

190106014726

vim-go does not highlight the returned types of b function, which are string and error.
It should be highlighted as goType syntax group (green color in this color scheme), but they actually have goSimpleArguments syntax group.

note: If let g:go_highlight_function_arguments = 0, it is highlighted below.

190106015724

configuration to reproduce

test.vim. Need to replace /path/to/vim-go/ with the actual path.

let g:go_highlight_function_arguments = 1
set nocompatible
set rtp^=/path/to/vim-go/
syntax on

test.go

package main

type T struct{}

func a() error {
	return nil
}
func b(x int) (string, error) {
	return "", nil
}
func c(y bool, z T) (str string, err error) {
	return
}
$ vim --clean -u test.vim test.go

cause

goSImpleArtuments syntax matching definition has a problem.
It matches a function argument (e.g. (a int), (a int, b bool)).

it specifys nextgroup=goSimpleArguments, it means the group repeats itself. Return types with bracktes are concealed by this nextgroup definition.

func b(x int) (string, error) {
  //  ^^^^^^^                 goSimpleArguments
  //          ^^^^^^^^^^^^^^^ repeated goSimpleArguments

I'm not sure why the nextgroup is specified. I think it's unnecessary because I guess go syntax does not allow an argument list after an argument list.
If someone knows the reasons of the nextgroup, please tell me it 🙏

@bhcleek
Copy link
Collaborator

bhcleek commented Jan 5, 2019

Thank you for contributing 🙇

I think it's unnecessary because I guess go syntax does not allow an argument list after an argument list

Without this, named return values may not be highlighted correctly. Perhaps there's an additional fix needed to highlight named return values correctly 🤔 ref: #1587

Before this PR:
image

This PR:
image

@bhcleek
Copy link
Collaborator

bhcleek commented Jan 5, 2019

I've come up with an additional fix to address the named return value situation; you can leave this PR as-is, and I'll build on it.

Thanks again for contributing!

@bhcleek bhcleek merged commit d2c5043 into fatih:master Jan 5, 2019
@pocke pocke deleted the fix-highlight branch January 5, 2019 18:58
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