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

chore: godoc popup border style #2900

Merged
merged 2 commits into from
May 28, 2020
Merged

chore: godoc popup border style #2900

merged 2 commits into from
May 28, 2020

Conversation

zhengkai
Copy link
Contributor

godoc popup cames from g:go_doc_popup_window

what I changed:

  1. beautify popup border style if utf-8 supported
  2. add variable g:go_doc_popup_border to support custome border style

default style (original):

default style

default style if utf-8 supported:

utf-8 style

bounded style with let g:go_doc_popup_border = ['─', '│', '─', '│', '╭', '╮', '╯', '╰'] :

rounded style

Copy link
Collaborator

@bhcleek bhcleek left a comment

Choose a reason for hiding this comment

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

Is there any reason to add the new option to make the border customizable?

@zhengkai
Copy link
Contributor Author

Sorry, I don't know how to change it in other way ? I saw the original style is hard coded

\ 'borderchars': ['-','|','-','|','+','+','+','+'],

Maybe someone like rounded or other style ? so he/she can changed it simply by new option ?

@bhcleek
Copy link
Collaborator

bhcleek commented May 28, 2020

I think that we can do with out g:go_doc_popup_border and just use borderchars as you've defined it.

i.e.

let borderchars = ['-', '|', '-', '|', '+', '+', '+', '+']
if &encoding == "utf-8"
   let borderchars = ['─', '│', '─', '│', '┌', '┐', '┘', '└']
endif

is sufficient; I don't see a reason to support a custom border style with g:go_doc_popup_border.

@zhengkai
Copy link
Contributor Author

From my perspective, I like as many options as possible. They don’t waste any memory and CPU. The only drawback is that they need to be written. But I have already done it.

Not everyone needs it, but someone needs it.

Maybe we can look at the opinions of others. If all of you feel unnecessary, you can remove it ( or by me ? )

@bhcleek
Copy link
Collaborator

bhcleek commented May 28, 2020

Yes, please remove the new option. More options is unnecessary overhead and should only be added when it adds clear value, because every option adds complexity. Constraints are useful, and I only have so much time to debug, triage issues, and maintain vim-go.

@bhcleek bhcleek merged commit ce3e8bd into fatih:master May 28, 2020
bhcleek added a commit that referenced this pull request May 28, 2020
@bhcleek bhcleek added this to the vim-go 1.24 milestone May 28, 2020
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