Skip to content
This repository has been archived by the owner on Mar 6, 2023. It is now read-only.

Support let g:gen_tags#find_tool options #58

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

d4em0n
Copy link

@d4em0n d4em0n commented Jun 30, 2018

g:gen_tags#find_tool is a command that displays list filenames. When g:gen_tags#find_tool is set. gen_tags will use it for generating tags for list of filenames.

For example,

let g:gen_tags#find_tool = 'rg --files'

gen_tags will execute command rg --files | ctags -f /home/ramdhan/.cache/tags_dir/homeramdhanctftoolsgef/prj_tags -L- .
rg --files command (ripgrep) can be used too for exclude filename/folder in .gitignore. #44

  • Support ctags only, i'am not use gtags.

@jsfaint
Copy link
Owner

jsfaint commented Jul 2, 2018

Thanks for the contribution, but I can't merge it right now.
This PR is still too rough.

  1. It breaks on neovim.
  2. It only support ctags, I wish it can support gtags in the same time.
  3. It introduce the external dependence.
  4. It should works on different platform, like windows, linux and macOS.

Sorry, I don't have enough time to review and refine this PR, right now.
Please keep it open and I'll look into it when I have some spare time.

Thanks again.

@jsfaint
Copy link
Owner

jsfaint commented Jul 2, 2018

Oh, this PR is releated to #44, BTW

@d4em0n
Copy link
Author

d4em0n commented Jul 2, 2018

Why breaks neovim?, I'm using neovim, and it's works fine.
We can using find with exclude options or git ls-files instead of ripgrep. ripgrep works on windows too.
Thanks for your reply

@@ -172,7 +172,7 @@ function! s:job_start(cmd, ...) abort
let l:job.on_exit = a:1
endif

let l:job_id = jobstart(a:cmd, l:job)
let l:job_id = jobstart(join(a:cmd), l:job)
Copy link
Owner

Choose a reason for hiding this comment

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

Oh, sorry. I just give the wrong comment.
This change only works for neovim and break on vim

@jsfaint
Copy link
Owner

jsfaint commented Jul 2, 2018

You should use gen_tags#opt_converter() to convert the option (list and string are both supported)

@jsfaint
Copy link
Owner

jsfaint commented Jul 2, 2018

The gtags can use the file list as below

rg --files | gtags -f -

@@ -183,8 +183,8 @@ function! s:job_start(cmd, ...) abort
if a:0 != 0
let l:job.exit_cb = a:1
endif

let l:job_id = job_start(a:cmd, l:job)
let l:cmd = ['/bin/sh', '-c', join(a:cmd)]
Copy link
Owner

Choose a reason for hiding this comment

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

Why do you add /bin/sh here?
There is no sh on windows by default.

@d4em0n
Copy link
Author

d4em0n commented Jul 3, 2018

Check/Test the latest commit, i don't know if let l:cmd = ['cmd', '/c', join(a:cmd)] worked or not

@jsfaint
Copy link
Owner

jsfaint commented Jul 3, 2018

There is no need to add 'cmd' or 'sh'
The command can work directly, for example

call jobstart(['rg', '--files', '|', 'gtags', '-f', '-'])

@d4em0n
Copy link
Author

d4em0n commented Jul 3, 2018

In vim that's doesn't work, i tried this.
For example :call job_start('echo hello > /tmp/b') this command doesn't work. But this command work for me :call job_start(["/bin/sh", "-c", "echo hello > /tmp/b"]). That's why im using sh

@d4em0n
Copy link
Author

d4em0n commented Jul 3, 2018

:call job_start(["echo", "hello", ">", "/tmp/c"]) this command doesn't work too in vim.

@jsfaint
Copy link
Owner

jsfaint commented Jul 3, 2018

you can't redirect the stdout in job_start, because the stdout of job_start only appear in callback function.

@d4em0n
Copy link
Author

d4em0n commented Jul 3, 2018

Don't know if job_start works using pipe |. This command doesn't work for me :call job_start(["echo", "hello", "|", "nc", "localhost" , "8080"]). It's work using :call job_start(["/bin/sh", "-c", "echo hello | nc localhost 8080"]). CMIIW

@jsfaint
Copy link
Owner

jsfaint commented Jul 3, 2018

I tried this code snippet, it works on windows and linux with vim8.1/neovim

function! s:job_stdout(job_id, data, ...) abort
  if type(a:data) == 1 "string
    echomsg a:data
  elseif type(a:data) == 3 "list
    for l:item in a:data
      echomsg l:item
    endfor
  endif
endfunction

function! s:job_start(cmd, ...) abort
  if has('nvim')
    let l:job = {
          \ 'on_stdout': function('s:job_stdout'),
          \ }

    let l:job_id = jobstart(a:cmd, l:job)
  elseif has('job')
    let l:job = {
          \ 'out_cb': function('s:job_stdout'),
          \ }

    let l:job_id = job_start(a:cmd, l:job)
  endif

  return l:job_id
endfunction

let s:list = ['ag', '-g', '""', '|', 'gtags', '-f', '-', '-v']

echomsg system(join(s:list))

call s:job_start(s:list)

@jsfaint
Copy link
Owner

jsfaint commented Jul 3, 2018

image
oh... I got this error..

@d4em0n
Copy link
Author

d4em0n commented Jul 3, 2018

yea, it's works in vim using system command. Above snippet doesn't work for me if i remove system line. It's should works in vim using job_start (without system command). Can you call job_start using s:list as argument directly in vim in your machine ?. sorry if im wrong

@d4em0n
Copy link
Author

d4em0n commented Jul 3, 2018

How do you got that error ?

@jsfaint
Copy link
Owner

jsfaint commented Jul 3, 2018

I add a stderr callback to print the error message.
I tried your code, it works with git ls-files, I don't have rg installed so I don't test with rg.
I tested with ag, but it fail to work.

It's a little hard that the user set the find tool by themself correctly.
Maybe we should remove g:gen_tags#find_tool option, and make it built-in?

gen_tags.vim detects the existing commands and use them directly.
Support some commands like git ls-files, rg, ag, fd?

@d4em0n
Copy link
Author

d4em0n commented Jul 5, 2018

I don't want remove g:gen_tags#find_tool. But if user not setting gen_tags#find_tool then gen_tags will detect existing command (like ag, rg) and set g:gen_tags#find_tool automatically. What do you think?

@jsfaint
Copy link
Owner

jsfaint commented Jul 5, 2018

The current code only works for rg and git ls-files

Some user may not use the rg like me.
And both rg and git ls-files only works for the git repo.

If I merge this PR now, it will become my own dog food. I need to maintain it in future.

It's easy to introduce a new option, but it's hard to remove it or change the behavior later.
Sorry, I can't merge it until I find a better way to handle it.

@d4em0n
Copy link
Author

d4em0n commented Jul 5, 2018

It's ok if you can't merge it. I don't want my code breaks this repo.
Do you want to close this PR or keep it open until this PR better?.
Feel free to ask me if you need help.

@jsfaint
Copy link
Owner

jsfaint commented Jul 5, 2018

Please keep it open, thanks anyway

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants