marshallward / vim-restructuredtext

Syntax file for reStructuredText on Vim.
26 stars 12 forks source link

Could italic and bold be, by default, highlighted as italic and bold? #4

Closed anntzer closed 8 years ago

anntzer commented 8 years ago

You have a note in syntax/rst.vim:

" TODO Append these atttributes somehow
"hi def rstEmphasis term=italic cterm=italic gui=italic
"hi def rstStrongEmphasis term=bold cterm=bold gui=bold

The following implementation (copied from the default syntax/markdown.vim)

hi def link rstEmphasis                     htmlItalic
hi def link rstStrongEmphasis               htmlBold

works for me.

marshallward commented 8 years ago

This isn't working for me unfortunately, nor does the Markdown syntax. I get some sort of bold highlight (as bold) but not the italics.

I think this may have something to do with my terminal program (Terminator), or maybe its my font (Liberation Sans Mono), which must be ignoring italic escape codes.

Ideally I'd like to use the normal coloring for terminals but set italics for the guis. But it also seems to be a good idea to support as many environments as possible.

I did look into modifying an existling highlight, but it seemed very difficult (if not impossible) to make any progress here.

I guess we could add another user flag...?

anntzer commented 8 years ago

With Konsole (KDE) and the Consolas font, I get italics with no problems in terminal vim.

marshallward commented 8 years ago

I just tested this in Termite and urxvt and both correctly use italics when I use term=italic. Terminator doesn't work with either. So I guess this has to do with the terminal emulator.

This list of supported terminals is probably relevant:

http://superuser.com/questions/204743/terminal-that-supports-the-ansi-italic-escape-code

(It looks like Markdown pulls its htmlItalic from syntax/html.vim, which just matches the same one in syntax/rst.vim).

I am inclined to support the lowest common denominator here and not assume italic escape code support, but it's a pretty basic feature. I could go either way here.

anntzer commented 8 years ago

I feel like if html.vim assumes it works it's fair just to do the same here, but obviously it's up to you...

blueyed commented 8 years ago

Check out https://github.com/neomake/neomake/blob/ab22f656cd3ce77a7092568c412b7422c15889e8/autoload/neomake/utils.vim#L177-L201 to get properties from existing highlight groups. This can be used like this: https://github.com/neomake/neomake/blob/27b803148dd16f48d1737acb4446ef1a1126ba52/autoload/neomake/signs.vim#L232-L261

marshallward commented 8 years ago

Wow! Exactly what I was looking for, thanks!

I'll try to throw something together that adds italic/bold to Underlined and Special.

(Or actually maybe just add the colours to htmlItalic and htmlBold)

marshallward commented 8 years ago

But of course it was easier to just add the ctermfg colours to the existing highlights, which is what I should have done all along...

Either way, I think this starts to resolve the issue. Antony, does it work if I just append ctermfg colours to the html highlights? I've updated the repo

In the future, maybe we can utilise the synIDattr function to fetch these colours from existing "safe" highlights. But this should be no less safe than what was already up there.

anntzer commented 8 years ago

Yes, works for me. Thanks.

rhysd commented 6 years ago

Since there is no reasonable way to know the terminal supports italic or not, I want to suggest switching highlight with variable.

if get(g:, 'rst_italic_bold_default_colors', 1)
  hi def rstEmphasis ctermfg=13 term=italic cterm=italic gui=italic
  hi def rstStrongEmphasis ctermfg=1 term=bold cterm=bold gui=bold
else
  hi def rstEmphasis                          term=italic cterm=italic gui=italic
  hi def rstStrongEmphasis                    term=bold cterm=bold gui=bold
endif

Then user can easily switch how to highlight the colors, I think.

marshallward commented 6 years ago

This is good for me. After reviewing this problem, I am not so keen on setting ctermfg anymore, since there's no precedent for it and there's no way to predict one's individual terminal colors anyway. Also, my terminal emulator of choice at that time (Terminator) now supports italics, so that issue is not even present anymore.

There still might be some users with terminals lacking italics, so no harm in leaving it in.

It's getting on midnight here, but if you send this (or just amend the existing PR) then I will merge it in tomorrow. I will probably send an update to Bram this week, since it is long overdue.

The only thing I'd suggest is maybe do if exists(...) instead of if get(...), unless there's some drawback to using exists that I don't know about.

rhysd commented 6 years ago

I'm sorry for late response since I was not responsible last weekend. Thank you for creating a PR at my fork. I'll check/merge into my PR branch.