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

[RFC] Man Plugin: Improve C highlighting #8709

Merged
merged 1 commit into from Jul 11, 2018

Conversation

Projects
None yet
6 participants
@Synray
Contributor

Synray commented Jul 8, 2018

I added a new region for Example sections because those sections are usually a page long, while Synopsis sections are usually a few lines. This should only sync to the start of EXAMPLE[S]

I'm not sure if group[t]here works.

You can see it in action on all man pages with Example sections with this shell command:

MANPAGER="nvim -c 'set ft=man' -" man -IKs 0:2:3 EXAMPLE

@marvim marvim added the RFC label Jul 8, 2018

@mhinz

This comment has been minimized.

Member

mhinz commented Jul 9, 2018

I'm unsure about this. :-)

It doesn't work on macOS for some reason. I hadn't the time to check why yet. (Highlighting the SYNOPSIS section does work.) That's no show-stopper, though.

It works in my Ubuntu VM, but it can lead to highlighting the text between the code blocks as C. (E.g. in printf(3))

So, do we prefer a pretty colorful section or no highlighting at all?

@Synray

This comment has been minimized.

Contributor

Synray commented Jul 10, 2018

I'm pretty sure it's a syntax syncing issue, but I'm not sure if I'm using syntax sync group[t]here right. grouphere doesn't seem to affect highlighting at all.

If you :syn sync minlines=300, the highlighting consistently works, but that seems like a suboptimal solution. The reason I'm separating out Examples and using grouphere is so that only what needs to be highlighted gets synced. Then again I'm not sure if C syntax highlighting is really that slow.

If performance isn't an issue, the solution would be to add EXAMPLES\= to the SYNOPSIS regex that is already there.

As for the wrong highlighted English in the Example section, I like the highlighted code more than I dislike the bright yellow for and etc. :)

csyntax

@mhinz

This comment has been minimized.

Member

mhinz commented Jul 10, 2018

If performance isn't an issue, the solution would be to add EXAMPLES= to the SYNOPSIS regex that is already there.

I'd prefer that over these groupthere hacks. :> I can confirm that it works.

And I have no strong opinion about having colored EXAMPLE sections (including all common text) or not. I hope that someone else decides.

@justinmk

This comment has been minimized.

Member

justinmk commented Jul 10, 2018

The highlighting in the screenshot is somewhat random. So it looks impressive at a glance, but adds noise to the non-code text.

To avoid false positives, maybe don't highlight lines that start with a Title case string. That might be good enough.

@mhinz

This comment has been minimized.

Member

mhinz commented Jul 10, 2018

I changed my mind and actually prefer the way the PR currently works now.

Because having an extra syntax group manExample, opposed to adding EXAMPLES\= to manSynopsis, means I can easily disable the syntax highlighting for that section, in case I ever decide not to like the false highlighting of normal text.

@Synray Synray force-pushed the Synray:man_syntax branch 2 times, most recently from ea48ce1 to efa1214 Jul 11, 2018

@Synray

This comment has been minimized.

Contributor

Synray commented Jul 11, 2018

I added a slightly conservative sentence match. Here's printf(3) again.
The red highlighting is not from any changes in this PR. I changed my :hi manUnderline to a red color, since I just found out that the script that preprocesses man pages, runtime/lua/man.lua, uses manUnderline. :)
csyntax2

@Synray Synray force-pushed the Synray:man_syntax branch from efa1214 to df43e63 Jul 11, 2018

@justinmk justinmk merged commit 07499a8 into neovim:master Jul 11, 2018

3 of 5 checks passed

QuickBuild Build pr-8709 finished with status FAILED
Details
codecov/project 81.44% (-0.01%) compared to 0ed8b12
Details
codecov/patch Coverage not affected when comparing 0ed8b12...df43e63
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@justinmk justinmk removed the RFC label Jul 11, 2018

justinmk added a commit that referenced this pull request Jul 18, 2018

NVIM v0.3.1
FEATURES:
07499a8 #8709 man.vim: C highlighting for EXAMPLES section
07f82ad #8699 TUI: urxvt: also send xterm focus-reporting seqs
40911e4 #8616 API: emit nvim_buf_lines_event from :terminal
c46997a #8546 fillchars: Add "eob" flag

FIXES:
74d19f6 #8576 startup: avoid blank stdin buffer if other files were opened
4874214 #8737 Only waitpid() for processes that we care about
cd6e7e8 #8743 Check all child processes for exit in SIGCHLD handler
c230ef2 #8746 channel.c: Prevent channel_destroy_early() from freeing uninitialized rpc stuff
0ed8b12 #8681 transstr_buf: fix length comparison
d241f27 #8708 TUI: Fix standout mode
9afed40 #8698 man.vim: fix for mandoc
e889640 #8682 provider/node: npm --loglevel silent
1cbc830 #8613 API: nvim_win_set_cursor: set curswant
bf6048e #8628 checkhealth: Python: fix VIRTUAL_ENV check
3cc3506 #8528 checkhealth: node.js: also search yarn

CHANGES:
b751449 #8619 defaults: shortmess+=F
1248178 #8578 highlight: high-priority CursorLine if fg is set.
01570f1 #8726 terminal: handle &confirm and :confirm on unloading
56065bb #8721 screen: truncate showmode messages
bf2460e #7551 buffer: fix copying :setlocal options
c1c14fa #8520 Ex mode: always "improved" (gQ)
050f397 #7992 options: remove 'maxcombine` option (always 6)

INTERNAL:
463da84 #7992 screen: use UTF-8 representation
@gravndal

This comment has been minimized.

gravndal commented Jul 19, 2018

This merge results in slightly odd highlighting for strcat(3).

Before above after:
2018-07-19-17 20 10

A short one word sentence added after program source:
2018-07-19-17 23 20

On a smaller terminal (without reflowing the manpage):
2018-07-19-17 24 01

And sorry for the excessively large pictures I guess, t̶h̶e̶ ̶c̶u̶l̶p̶r̶i̶t̶ ̶i̶s̶ ̶t̶h̶e̶ ̶̶P̶r̶o̶g̶r̶a̶m̶ ̶s̶o̶u̶r̶c̶e̶̶ ̶l̶i̶n̶e̶.̶

@justinmk

This comment has been minimized.

Member

justinmk commented Jul 19, 2018

@gravndal You mean the "Program source" header isn't highlighted? Otherwise I don't see a problem.

@ilAYAli

This comment has been minimized.

ilAYAli commented Jul 19, 2018

Example code for c++ manpages (e.g std::vector) are located in a section called Example, so that might also be worth including in the syntax region.

@gravndal

This comment has been minimized.

gravndal commented Jul 19, 2018

@justinmk No meaning that Program source was the line that caused issues with the highlighting, e.g. SEE ALSO not being highlighted, or the C code not being highlighted unless it is out of view (and a syntax update is made/forced). On second review, deleting it doesn't change anything so that was a bit of an early conclusion on my end.

The highlighting of the last line is also somewhat sporadic, though that doesn't have anything to do with this pull as far as I can tell.

@justinmk

This comment has been minimized.

Member

justinmk commented Jul 19, 2018

@gravndal Your very last screenshot shows what looks like the desired syntax highlighting. If bigger screen or scrolling degrades the experience, it's most likely a syntax performance issue, not related to this PR. you can try:

:syntax sync fromstart

also try CTRL-L .

@gravndal

This comment has been minimized.

gravndal commented Jul 19, 2018

@justinmk Tried that, it doesn't change anything. What does however is putting the cursor on top of one of the brackets in the example code, the result being highlighting up and until the matching bracket. Doing a CTRL-L or :syntax sync fromstart will then remove the highlighting.

(also tried without my config too rule out anything to do with non defaults, yields the same result)

@justinmk

This comment has been minimized.

Member

justinmk commented Jul 19, 2018

Can't reproduce. In any case, this PR was a "best effort". As long as it didn't seriously regress anything, please send improvements in the form of a PR. It's not clear from the images if anything seriously regressed.

@gravndal

This comment has been minimized.

gravndal commented Jul 19, 2018

So playing around with it a bit more, I'd say it's the sentence check. Adding a dot after Program source, or deleting the line and the parenthesis after ... performance.) brings the desired highlighting.

@Synray

This comment has been minimized.

Contributor

Synray commented Jul 20, 2018

deleting the line and the parenthesis after ... performance.) brings the desired highlighting.

That makes sense. Since a .) shouldn't occur in C code, I'll try adding .) to the pattern.

@justinmk

This comment has been minimized.

Member

justinmk commented Jul 20, 2018

@Synray it might be best to check if there is a line immediately below the title-cased match. If not, then it's probably not a sentence.

@Synray

This comment has been minimized.

Contributor

Synray commented Jul 20, 2018

check if there is a line immediately below the title-cased match.

That is way simpler and seems to work better. I made manSentence a region:

  syntax region manSentence start=/^\s\{3,7}\%(\u\|\*\)[^{}=]*/ end=/\n$/ end=/\ze\n\s\{3,7}#/ keepend contained contains=manReference

I also added manLowerSentence:

  syntax match manLowerSentence /\n\s\{3,7}\l.\+[()]\=\%(\:\|.\|-\)[()]\=[{};]\@<!\n$/ display keepend contained contains=manReference

manLowerSentence matches single-line sentence fragments like those in printf(3).

In the example screenshot I highlighted manSentence and manLowerSentence with a brighter background -- manLowerSentence is highlighted with twice the intensity.
csyntax4
And here's strcat(3):
csyntax-strcat3
These regions hide manSubHeading, which could be prevented if I only match sentences that start after 7 spaces, by changing the \{3,7} part of the match to \{7}, but I recall seeing sentences indented at 3 spaces in a man page that I forget the name of.

@Synray Synray deleted the Synray:man_syntax branch Jul 20, 2018

@gravndal

This comment has been minimized.

gravndal commented Jul 20, 2018

As far as I can tell by the defaults laid out in groff_man(7) the only lines that will have 3n indentation are lines created with the .SS macro (described as subsections in man-pages(7) and sub-subheadings in groff_man(7)). While some subheadings might be quite sentence-like aren't they still subheadings?

@Synray

This comment has been minimized.

Contributor

Synray commented Jul 20, 2018

described as subsections in man-pages(7)

In that case then they should be highlighted as sub headings. I'll make the change.

justinmk added a commit that referenced this pull request Jul 22, 2018

UtkarshMe added a commit to UtkarshMe/neovim that referenced this pull request Jul 28, 2018

UtkarshMe added a commit to UtkarshMe/neovim that referenced this pull request Jul 28, 2018

NVIM v0.3.1
FEATURES:
07499a8 neovim#8709 man.vim: C highlighting for EXAMPLES section
07f82ad neovim#8699 TUI: urxvt: also send xterm focus-reporting seqs
40911e4 neovim#8616 API: emit nvim_buf_lines_event from :terminal
c46997a neovim#8546 fillchars: Add "eob" flag

FIXES:
74d19f6 neovim#8576 startup: avoid blank stdin buffer if other files were opened
4874214 neovim#8737 Only waitpid() for processes that we care about
cd6e7e8 neovim#8743 Check all child processes for exit in SIGCHLD handler
c230ef2 neovim#8746 channel.c: Prevent channel_destroy_early() from freeing uninitialized rpc stuff
0ed8b12 neovim#8681 transstr_buf: fix length comparison
d241f27 neovim#8708 TUI: Fix standout mode
9afed40 neovim#8698 man.vim: fix for mandoc
e889640 neovim#8682 provider/node: npm --loglevel silent
1cbc830 neovim#8613 API: nvim_win_set_cursor: set curswant
bf6048e neovim#8628 checkhealth: Python: fix VIRTUAL_ENV check
3cc3506 neovim#8528 checkhealth: node.js: also search yarn

CHANGES:
b751449 neovim#8619 defaults: shortmess+=F
1248178 neovim#8578 highlight: high-priority CursorLine if fg is set.
01570f1 neovim#8726 terminal: handle &confirm and :confirm on unloading
56065bb neovim#8721 screen: truncate showmode messages
bf2460e neovim#7551 buffer: fix copying :setlocal options
c1c14fa neovim#8520 Ex mode: always "improved" (gQ)
050f397 neovim#7992 options: remove 'maxcombine` option (always 6)

INTERNAL:
463da84 neovim#7992 screen: use UTF-8 representation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment