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] Set 'keywordprg' for vim and help filetypes. #3104

Closed
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
9 participants
@lithammer
Contributor

lithammer commented Jul 26, 2015

Open the Vim help file instead of man when pressing K in normal mode.

@lithammer

This comment has been minimized.

Contributor

lithammer commented Jul 26, 2015

I guess this should have the defaults label.

@vheon

This comment has been minimized.

Contributor

vheon commented Jul 26, 2015

I would prefer this behaviour from tpope/vim-scriptease as a default for vim files.

@justinmk

This comment has been minimized.

Member

justinmk commented Jul 26, 2015

Yeah, I was thinking scriptease should be a default plugin, actually. (Please don't send a PR for that, though...)

@vheon Any reason this PR would be a problem in the meantime? If users have scripttease they won't notice, and users that don't have scriptease get some benefit.

@lithammer

This comment has been minimized.

Contributor

lithammer commented Jul 26, 2015

What does that scriptease snippet actually do? It wasn't immediately obvious to me.

@fmoralesc

This comment has been minimized.

Contributor

fmoralesc commented Jul 26, 2015

@renstrom It adjusts the help-topic depending on what's under the cursor.

@lithammer

This comment has been minimized.

Contributor

lithammer commented Jul 26, 2015

Ah, so it's just a bit more intelligent than this solution?

@vheon

This comment has been minimized.

Contributor

vheon commented Jul 26, 2015

@renstrom yes

@justinmk no, absolutely. I was just thinking to do it right once, since the solution is already available.

@ghost ghost changed the title from Set 'keywordprg' for vim and help filetypes. to [RFC] Set 'keywordprg' for vim and help filetypes. Jul 26, 2015

@ghost

This comment has been minimized.

ghost commented Jul 26, 2015

This should be mentioned in vim_diff.txt.

Besides that I've used this for a while, and while it mostly works fine I've always had the following issue:

  • open empty buffer, :setf help
  • create the buffer contents |help-context|
  • put cursor on first letter and press K, taken to *help* tag
  • put cursor on c and press K, taken to *:syn-context* tag

I just tried :set iskeyword+=- and it seems to fix it, but I'm not sure if it will work in all cases.

@marvim marvim added the RFC label Jul 26, 2015

@fmoralesc

This comment has been minimized.

Contributor

fmoralesc commented Jul 26, 2015

@Pyrohh It's weird you had to do that, since the help on 'iskeyword' says:

For a help file it is set to all non-blank printable characters except
'*', '"' and '|' (so that CTRL-] on a command finds the help for that
command).

I think that is not true.

@lithammer

This comment has been minimized.

Contributor

lithammer commented Jul 27, 2015

In the case of using the scriptease version, where would one put the s:helptopic() function?

@fmoralesc

This comment has been minimized.

Contributor

fmoralesc commented Jul 27, 2015

I would suggest you put it in some file in runtime/autoload, so you can call it from both vim and help files, and it only gets defined on demand.

You'll also need to modify it so it can work standalone, as it stands it uses some other stuff from scriptease.

@lithammer lithammer force-pushed the lithammer:set-keywordprg-for-vim-and-help branch 2 times, most recently from 949e950 to 34b9794 Sep 16, 2015

@lithammer

This comment has been minimized.

Contributor

lithammer commented Sep 16, 2015

Ported the functionality from vim-scriptease.

return reverse(map(synstack(line, col), 'synIDattr(v:val,"name")'))
endfunction
function! HelpTopic()

This comment has been minimized.

@ZyX-I

ZyX-I Sep 16, 2015

Contributor

Since you are using autoload file you must use help#topic() (help is because file is named {rtp}/autoload/help.vim).

This comment has been minimized.

@ZyX-I

ZyX-I Sep 16, 2015

Contributor

Also missing abort.

@@ -0,0 +1,38 @@
function! s:synnames(...) abort

This comment has been minimized.

@ZyX-I

ZyX-I Sep 16, 2015

Contributor

Missing guard.

This comment has been minimized.

@justinmk

justinmk Sep 16, 2015

Member

Guard for what? I don't think autoload files need a guard.

This comment has been minimized.

@ZyX-I

ZyX-I Sep 16, 2015

Contributor

@justinmk They have it. E.g. look at netrw.vim.

else
let [line, col] = [line('.'), col('.')]
endif
return reverse(map(synstack(line, col), 'synIDattr(v:val,"name")'))

This comment has been minimized.

@ZyX-I

ZyX-I Sep 16, 2015

Contributor

Missing space after comma.

let syn = get(synnames(), 0, '')
let cword = expand('<cword>')
if syn ==# 'vimFuncName'
return cword.'()'

This comment has been minimized.

@ZyX-I

ZyX-I Sep 16, 2015

Contributor

Here and below: missing space around operator.

@@ -20,3 +20,6 @@ endif
let &cpo = s:cpo_save
unlet s:cpo_save
" Open the help file when pressing 'K'.
nnoremap <silent><buffer> K :exe 'help '.HelpTopic()<CR>

This comment has been minimized.

@ZyX-I

ZyX-I Sep 16, 2015

Contributor

This is better expressed as

:execute 'help' help#topic()<CR>

(note: no space after help in a string, no concat operator).

@@ -75,3 +75,6 @@ unlet s:cpo_save
" removed this, because 'cpoptions' is a global option.
" setlocal cpo+=M " makes \%( match \)
" Open the help file when pressing 'K'.
nnoremap <silent><buffer> K :exe 'help '.HelpTopic()<CR>

This comment has been minimized.

@ZyX-I

ZyX-I Sep 16, 2015

Contributor

Note that without using autoloaded functions naming convention (i.e. autoload/path/to/file.vim transforms into functions looking like path#to#file#funcname) all this code does is displaying E117 error.

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented Sep 16, 2015

@neovim/core Also what about tests? In #3270 I assumed that it is bad idea to add any code without testing and put tests for runtime files into test/functional/plugin. I think it is needed to establish an agreement on this matter. E.g. for this issue it needs to test that K opened in help and vim files opens right topics.

let post = getline('.')[col : -1]
let syn = get(synnames(), 0, '')
let cword = expand('<cword>')
if syn ==# 'vimFuncName'

This comment has been minimized.

@ZyX-I

ZyX-I Sep 16, 2015

Contributor

The entire function is for .vim files: this and the following conditions will never match in a help file. I think this needs to be altered: topic for functions should be opened if current word is immediately followed by a parenthesis in addition to syn check.

let cword = expand('<cword>')
if syn ==# 'vimFuncName'
return cword.'()'
elseif syn ==# 'vimOption'

This comment has been minimized.

@ZyX-I

ZyX-I Sep 16, 2015

Contributor

Topic for options: if current word is preceded with &, &l: or &g: or surrounded by '. Also if some preceding text contains set, setl[ocal] or setg[lobal]. Note: only in the latter case it sometimes uses vimOption syntax group (sometimes: because it may be execute 'set rtp='.…). E.g. for let &syntax='foo' it will not guess that it needs to open 'syntax'.

return cword.'()'
elseif syn ==# 'vimOption'
return "'".cword."'"
elseif syn ==# 'vimUserAttrbKey'

This comment has been minimized.

@ZyX-I

ZyX-I Sep 16, 2015

Contributor

If word is preceded with - and there is com[mand] somewhere behind.

return "'".cword."'"
elseif syn ==# 'vimUserAttrbKey'
return ':command-'.cword
elseif pre =~# '^\s*:\=$'

This comment has been minimized.

@ZyX-I

ZyX-I Sep 16, 2015

Contributor

Specifically for vim files open help with : if word happens to contain the first non-whitespace characters on the current line, or is preceded with : (there is a common convention to write autocmd BufWritePre * :command, same for some other complex commands like :command itself). Specifically for help files leave only the condition that current word is preceded with a colon.

return ':command-'.cword
elseif pre =~# '^\s*:\=$'
return ':'.cword
elseif pre =~# '\<v:$'

This comment has been minimized.

@ZyX-I

ZyX-I Sep 16, 2015

Contributor

Replace v with [vgbtw]. Plugins are configured using variables and they add documentation with prefix: e.g. :h g:netrw_ftp.

return ':'.cword
elseif pre =~# '\<v:$'
return 'v:'.cword
elseif cword ==# 'v' && post =~# ':\w\+'

This comment has been minimized.

@ZyX-I

ZyX-I Sep 16, 2015

Contributor

Same.

let col += 1
endwhile
let post = getline('.')[col : -1]
let syn = get(synnames(), 0, '')

This comment has been minimized.

@ZyX-I

ZyX-I Sep 16, 2015

Contributor

Missing s: before synnames. Also you do not need this function at all: simply use synIDattr(synID(line('.'), col('.')), 'name'), you are using only very small part of the s:synnames functionality.

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented Sep 16, 2015

By the way, why using help#topic() for help files at all? Simple nnoremap <buffer> K <C-]> should be enough here.

@justinmk

This comment has been minimized.

Member

justinmk commented Sep 16, 2015

what about tests? In #3270 I assumed that it is bad idea to add any code without testing and put tests for runtime files into test/functional/plugin

Yes, we prefer tests. And test/functional/plugin/help_spec.lua would be a good place for it.

@lithammer lithammer referenced this pull request Sep 23, 2015

Merged

[RFC] use :Man instead of `man` for K #1878

6 of 6 tasks complete

@lithammer lithammer force-pushed the lithammer:set-keywordprg-for-vim-and-help branch from eaa1533 to ce6d317 Oct 18, 2015

@josephholsten

This comment has been minimized.

Contributor

josephholsten commented Jul 7, 2017

@justinmk @ZyX-I @renstrom what's remaining to get this completed? I'm personally happy with autocmd Filetype vim setlocal keywordprg=:help, but this fancier impl seems close.

Peter Renström added some commits Jul 26, 2015

Peter Renström
Set keywordprg for vim and help filetypes.
Open the Vim help file instead of 'man' when pressing 'K' in normal
mode.

Credits to Tim Pope for the original implementation in vim-scriptease.

@lithammer lithammer force-pushed the lithammer:set-keywordprg-for-vim-and-help branch from 759316d to a13ea1d Jul 7, 2017

@lithammer

This comment has been minimized.

Contributor

lithammer commented Jul 7, 2017

I tried to address some comments and rebased since this was two years old. Unfortunately I'm quite bad a Vimscript.

return "'" . cword . "'"
elseif pre =~# '&$' || pre =~# '&[lg]:$'
return "'" . cword . "'"
elseif pre =~# 'set\s\+$' || pre =~# '\vsetl(ocal)?\s+$' || pre =~# '\vsetg(lobal)?\s+$'

This comment has been minimized.

@mhinz

mhinz Jul 8, 2017

Member

This matches setl and setlocal, but not setloc. Use this instead:

elseif pre =~# '\vset?\s+$' || pre =~# '\vsetl%[ocal]\s+$' || pre =~# '\vsetg%[lobal]\s+$'
@josephholsten

This comment has been minimized.

josephholsten commented on runtime/autoload/help.vim in a33f027 Jul 8, 2017

Can we squash this into the initial patch? Any reason to keep it separate?

This comment has been minimized.

Owner

lithammer replied Jul 8, 2017

I just wanted to address each comment with one commit. All this can be squashed later when it's complete and ready to merge.

This comment has been minimized.

josephholsten replied Jul 9, 2017

👍

endif
let g:loaded_help = 1
function! help#topic() abort

This comment has been minimized.

@josephholsten

josephholsten Jul 8, 2017

Contributor

Is this function obvious enough that it doesn't need a comment? I'm too green to judge. Would it qualify as obvious if it had tests?

This comment has been minimized.

@lithammer

lithammer Jul 8, 2017

Contributor

Honestly, I barely know myself since this was blatantly copied from https://github.com/tpope/vim-scriptease/blob/59a73a2415ea1b006ae3e91163ddde24e8540844/autoload/scriptease.vim#L693-L723. Initially this pull request only contained a simple keywordprg=:help, but then it was suggested to use vim-scriptease's more intelligent lookup.

But in short, this function looks at the word under the cursor (e.g. what syntax group it has), and on the word(s) coming before and after.

@justinmk

This comment has been minimized.

Member

justinmk commented Jul 8, 2017

Contrary to #3104 (comment) I think it's now a good time to include scriptease as a standard plugin. @tpope do you have any objection to that?

@lithammer

This comment has been minimized.

Contributor

lithammer commented Jul 8, 2017

What does it really mean to include as a standard plugin?

@justinmk

This comment has been minimized.

Member

justinmk commented Jul 8, 2017

@renstrom It means include the plugin in runtime/pack/dist/start/scriptease

@justinmk

This comment has been minimized.

Member

justinmk commented Jul 8, 2017

6720fe2 includes a minimal version of this change.

A new PR including runtime/pack/dist/start/scriptease would be welcome. We would like to establish a workflow for including more plugins as standard plugins in runtime/pack/dist/*, possibly as git subtrees. Help there is welcome.

@justinmk justinmk closed this Jul 8, 2017

@justinmk justinmk removed the RFC label Jul 8, 2017

justinmk added a commit that referenced this pull request Jul 8, 2017

@lithammer lithammer deleted the lithammer:set-keywordprg-for-vim-and-help branch Jul 8, 2017

@tpope

This comment has been minimized.

tpope commented Jul 8, 2017

No objection. I would recommend sticking to stable releases rather than tracking master, though with Neovim still in development maybe that doesn't matter.

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented Jul 8, 2017

Neovim is indeed in development and here we at least wait for CI before merging patches, so I would rather suggest master.

justinmk added a commit to justinmk/neovim that referenced this pull request Nov 8, 2017

NVIM v0.2.1
FEATURES:
0e873a3 Lua(Jit) built-in neovim#4411
5b32bce Windows: `:terminal` neovim#7007
7b0ceb3 UI/API: externalize cmdline neovim#7173
b67f58b UI/API: externalize wildmenu neovim#7454
b23aa1c UI: 'winhighlight' neovim#6597
17531ed UI: command-line coloring (`:help input()-highlight`) neovim#6364
244a1f9 API: execute lua directly from the remote api neovim#6704
45626de API: `get_keymap()` neovim#6236
db99982 API: `nvim_get_hl_by_name()`, `nvim_get_hl_by_id()` neovim#7082
dc68538 menu_get() function neovim#6322
9db42d4 :cquit : take an error code argument neovim#7336
9cc185d job-control: serverstart(): support ipv6 neovim#6680
1b7a9bf job-control: sockopen() neovim#6594
6efe84a clipboard: fallback to tmux clipboard neovim#6894
6016ac2 clipboard: customize clipboard with `g:clipboard` neovim#6030
3a86dd5 ruby: override ruby host via `g:ruby_host_prog` neovim#6841
16cce1a debug: $NVIM_LOG_FILE neovim#6827
0cba3da `:checkhealth` built-in, validates $VIMRUNTIME neovim#7399

FIXES:
105d680 TUI: more terminals, improve scroll/resize neovim#6816
cb912a3 :terminal : handle F1-F12, other keys neovim#7241
619838f inccommand: improve performance neovim#6949
04b3c32 inccommand: Fix matches for zero-width neovim#7487
60b1e8a inccommand: multiline, other fixes neovim#7315
f1f7f3b inccommand: Ignore leading modifiers in the command neovim#6967
1551f71 inccommand: fix 'gdefault' lockup neovim#7262
6338199 API: bufhl: support creating new groups neovim#7414
541dde3 API: allow K_EVENT during operator-pending
8c732f7 terminal: adjust for 'number' neovim#7440
5bec946 UI: preserve wildmenu during jobs/events neovim#7110
c349083 UI: disable 'lazyredraw' during ui_refresh. neovim#6259
51808a2 send FocusGained/FocusLost event instead of pseudokey neovim#7221
133f8bc shada: preserve unnamed register on restart neovim#4700
1b70a1d shada: avoid assertion on corrupt shada file neovim#6958
9f534f3 mksession: Restore tab-local working directory neovim#6859
de1084f fix buf_write() crash neovim#7140
7f76986 syntax: register 'Normal' highlight group neovim#6973
6e7a8c3 RPC: close channel if stream was closed neovim#7081
85f3084 clipboard: disallow recursion; show hint only once neovim#7203
8d1ccb6 clipboard: performance, avoid weird edge-cases neovim#7193
01487d4 'titleold' neovim#7358
01e53a5 Windows: better path-handling, separator (slash) hygiene neovim#7349
0f2873c Windows: multibyte startup arguments neovim#7060

CHANGES:
9ff0cc7 :terminal : start in normal-mode neovim#6808
032b088 lower priority of 'cursorcolumn', 'colorcolumn' neovim#7364
2a3bcd1 RPC: Don't delay notifications when request is pending neovim#6544
023f67c :terminal : Do not change 'number', 'relativenumber' neovim#6796
1ef2d76 socket.c: Disable Nagle's algorithm on TCP sockets neovim#6915
6720fe2 help: `K` tries Vim help instead of manpage neovim#3104
7068370 help, man.vim: change "outline" map to `gO` neovim#7405

@justinmk justinmk referenced this pull request Nov 8, 2017

Merged

NVIM v0.2.1 #7505

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment