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 :AckHelp command bug with missing *.txt files #182

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

Conversation

AndrewRadev
Copy link

If there are no *.txt files in one of the doc directories, the ack command errors out. Globbing for those files while generating the file list fixes the issue.

I've also added an extra improvement that I can remove if you'd prefer. The fnamemodify call shortens the file path as much as possible, so, if I'm running :AckHelp from within /home/andrew/.vim, instead of getting paths like /home/andrew/.vim/bundle/ack/..., I just get bundle/ack/.... This seems to make a difference to my quickfix window, at least.

If there are no *.txt files in the doc directory, the ack command errors
out. Globbing for those files while generating the file list fixes the
issue.
This improves the quickfix list filenames in the case when the found
files are under the current directory.
@ches
Copy link
Collaborator

ches commented May 3, 2016

Thanks! On the surface this looks great, but in testing it some problems are surfacing for me—I may need to further reduce my config, but let's see if you can corroborate any of this.

There are quite possibly extant issues to blame here that I just haven't personally noticed without changing settings from what I normally use, because just looking at this change it shouldn't seem to cause any problem. So please bear with me…

The main issue I'm seeing when using this change occurs with let g:ack_use_dispatch = 1 (using regular Vim FWIW, not NeoVim). It always seems to put Vim into pager mode once the results load into quickfix, i.e. it's prompting me to page down or q to quit paging. I hit q and then the original file window is focused instead of the visible quickfix list. With let g:ack_use_dispatch = 0 it's fine and this doesn't happen, and without this change in place (current master checked out), this also doesn't occur whether Dispatch support is enabled or not. I'm using Vim in the console in a tmux session, so Dispatch is using tmux. I'll try it with another of Dispatch's supported "strategies" and see if it behaves the same way.

By the way, to get it to behave even this nicely with Dispatch, I had to add --nopager to the plugin's default Ack call:

let g:ackprg = 'ack -s -H --nocolor --nogroup --column --nopager'

My personal ~/.ackrc has --pager=less, and this seems to interact poorly with g:ack_use_dispatch = 1—I've never noticed because I normally use let g:ackprg = 'ag --vimgrep' and that Ag option seems to disable pager by default. I think a case could be made that Ack should override a configured --pager option if it's not being run on a TTY, but that doesn't seem to be the case now so I'm thinking that adding an explicit --nopager to ack.vim's g:ack_default_options would be an improvement with no negative effect… Otherwise it's probably broken out of the box with default config modulo enabling g:ack_use_dispatch.

That's beside the point of this issue of course, just thinking out loud and also mentioning it in case you need to do the same to test with Dispatch…

So in summary, the first thing to try to resolve is that any time I have g:ack_use_dispatch enabled, whether with default g:ackprg (or --nopager added) or using ag for it, I'm being left in pager mode once results load. Of course that's an undesirable interruption.

ches added a commit that referenced this pull request May 3, 2016
Otherwise if a user has something like `--pager=less` as a default in
their `~/.ackrc`, Ack may think that it's running interactively if
executed in a separate shell e.g. when `g:ack_use_dispatch` is enabled.
This will result in broken behavior loading results in Vim.

See #182 for some background.
@ches
Copy link
Collaborator

ches commented May 3, 2016

Ugh, well, oddly it seems fine using Dispatch's iTerm support, I'm not experiencing the same pager interruption. I'd like to find some way to sort this out, since using Dispatch with tmux is how I personally run Vim primarily 😄 It is eluding me how the changes in this branch make any difference here 😩

Second, I seem to have problems with the fnamemodify commit too—I'm on board with that idea for more readable quickfix if it works. For me:

  • If I run :AckHelp! python when pwd is my home directory, I get results from help files beneath my ~/.vim directory, and the built-in help files in the Vim distribution's main runtime path, as expected.
  • If I cd ~/.vim, the same is true.
  • If I cd ~/.vim/after, I only get results from the built-in help files, no user help.
  • If I cd ~/Documents, the same is true, only built-in help results and no user help.

Can you reproduce this? The second commit that adds fnamemodify is the culprit, if I cherry-pick only the first commit these problems are gone.

@ches
Copy link
Collaborator

ches commented May 3, 2016

isdirectory() seems to return 0 for a path abbreviated with ~, perhaps adding expand() to that check is the solution.

Testing one runtimepath component when pwd is something like ~/src:

:let p = fnamemodify('/Users/ches/.vim/bundle/dash.vim/' . '/doc/', ':~:.')
:echo p  " => ~/.vim/bundle/dash.vim//doc/
:echo isdirectory(p)  " => 0
:echo isdirectory(expand(p))  " => 1

@AndrewRadev
Copy link
Author

Huh, I didn't expect this to be an issue. I've pushed a commit that uses expand like you suggested. I tested glob and it seems like that one doesn't have an issue with ~, so this should fix the problem, I hope.

@AndrewRadev
Copy link
Author

As for the pager, I don't really know. I don't use dispatch, and I don't have tmux set up, so it's a bit difficult to test it out (and I'm not on a Mac, so iTerm is also out :)). It seems weird that this particular change would affect the pager option. Maybe it's the glob function for some reason? I can't imagine why...

Adding --nopager seems sensible regardless. Does that fix the issue for you?

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.

None yet

2 participants