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 s:hi_normal initialization #74

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

iazel
Copy link

@iazel iazel commented Jun 27, 2014

Some schema has font=Monospaced 10, but because the regexp doesn't include a space, the 10 will hang around and break the color name -> hex conversion.
Just added a space.

This will resolve issue #67

Some schema has `font=Monospaced 10`, but because the regexp doesn't include a space, the 10 will hang around and break the color name -> hex conversion.
Just added a space.
@nathanaelkane
Copy link
Collaborator

I'm unable to replicate the problem that this PR fixes. Are you able to give me more details? Such as:

  • Operating System
  • GUI Vim or Terminal Vim
  • Vim version
  • Colorscheme
  • Link to your dotfiles (if possible)

When I put hi Normal font=Monospaced 10 into my vimrc it gives me the following error upon opening Vim:

Error detected while processing /home/vagrant/.vimrc:
line    3:
E416: missing equal sign: 10

Wrapping in single quotes seems to work on Windows, but it replaces all the spaces with underscores.

I'd like to merge this in, but I'd like to replicate the problem first. :)

@iazel
Copy link
Author

iazel commented Oct 19, 2014

To be totally honest, now I can't reproduce it either! I've upgraded vim since the commit, maybe something change. I use GVim on Ubuntu, with the desert color scheme.

Take in consideration that many other experienced the same issue.

@nathanaelkane
Copy link
Collaborator

All good, I'll try some older versions of Vim on Ubuntu.

Either way, it's exposed another bug with that font= regex on Windows.

@alerque
Copy link
Member

alerque commented Feb 21, 2023

I am also unable to replicate this bug, but having looked at the situation there would a problem with allowing spaces as per this PR. The font name can certainly include spaces, but in order to do so the string would need to be escaped somehow because the other segments in s:hi_normal are space separated. Unless we know that this value would always be last, I don't think this regex would work without being too greedy.

I'm happy to review this situation if anybody has more info, a working case to replicate the bug, or whatever feedback. In the mean time I'm marking this as draft because it's not actionable in this form.

@@ -185,7 +185,7 @@ function! indent_guides#init_script_vars()
let s:hi_normal = indent_guides#capture_highlight('Normal')

" remove 'font=<value>' from the s:hi_normal string (only seems to happen on Vim startup in Windows)
let s:hi_normal = substitute(s:hi_normal, ' font=[A-Za-z0-9:]\+', "", "")
let s:hi_normal = substitute(s:hi_normal, ' font=[A-Za-z0-9: ]\+', "", "")
Copy link
Member

Choose a reason for hiding this comment

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

This would be too greedy because other segments are space separated.
A proper match (if such a thing is even still needed at all) wourd need to
account for whatever quoting or escaping is used in this value. Since
I don't have a working test case I can't say what that needs to be.

@alerque alerque marked this pull request as draft February 21, 2023 13:43
@alerque
Copy link
Member

alerque commented Feb 21, 2023

It seems like this is related to #38 as well.

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.

3 participants