AndrewRadev / writable_search.vim

Grep for something, then write the original files directly through the search results.
http://www.vim.org/scripts/script.php?script_id=4865
MIT License
51 stars 3 forks source link

Writable search from quickfix #6

Closed p0deje closed 8 years ago

p0deje commented 9 years ago

Hi and thanks for great plugin!

I'd like to implement the following flow and want to know if you think that such pull request can be merged.

  1. Using ag.vim (or :Ggrep, etc.), find something.
  2. Load results into quickfix window (because it's easier to see the matched lines and navigate through them).
  3. Execute :WritableSearch in quickfix window. That will create new writable_search buffer to apply any changes.

To me, that flow makes sense and is useful in many ways (especially if I find the way to expand matched lines with surrounded text). What do you think about it? Would you mind seeing pull request for that?

AndrewRadev commented 9 years ago

I'd love a PR that implements a to/from quickfix functionality! :) I mentioned that in a different issue, #5, though most of it is on a different topic.

I've been thinking of working on this myself, but I haven't had the time, so a feature like this would be welcome. There are a few issues I've thought about, though.

For starters, I really don't want to remove the existing workflow. I like the idea of WritableSearch query working directly to open a new buffer. If nothing else, there's some logic implemented for different backends which I definitely don't want to lose.

In my personal experience, I've encountered some cases when I want to open a writable search buffer, and some when I want to move to a quickfix window, and the other way around. So I think it would be best to have both functionalities, like :WritableSearchFromQuickfix and :WritableSearchToQuickfix, although I don't really like the names that much. Of course, there's no harm in starting from just one of the features :). I'd just like to have both, ultimately.

A practical issue is the implementation. Currently, the process goes:

Taking them from the quickfix window would mean that you get no context :/. The :Ack or :Ag commands just run the query without -C3 and end up with a single line per result. Which is fine, because that's what the quickfix window expects. But I think that usually, if you're seeing the writable search buffer, you want to have at least a couple of lines of context. I think you've figured out this problem as well, but I'll expand on it.

The other way around has the same issue. You get a batch of lines surrounding the results, but you only want to take the relevant lines and plug them in the quickfix window.

In one direction, if you're taking the writable search buffer, and creating a quickfix window from that, you could do that relatively easily by just rerunning the query with the -C<n> removed from the flags, so you only get one result per line. And then parse it and load it in the quickfix window. In the other direction, it's trickier. The quickfix window doesn't remember what query was run to generate it. In fact, it may not have been a query at all, could have been just setqflist().

So I think in this case, it would make sense instead to take the lines from the quickfix window and read in context lines from the files using Vimscript. Some care would need to be taken to merge the results -- for instance, if you have:

result line: 14
result line: 15

The contextual result would have to be:

context line 13
result line: 14
result line: 15
context line: 16

So, the two results would have to be "merged" in the writable search buffer in a single block (which is usually done by the grep or ack command. I'm not sure if I'm explaining this clearly enough, let me know).

In any case, I'm sure it would be doable, it would just take some code to get it right, I think. If you don't feel like working on it, you could even just do it with one-line-per result for now, and I'll think about adding the context later (maybe with your help, if you're up to it). Having the to/from quickfix interface would be some start at least.

Let me know if you need help with navigating the code, or you have any questions or uncertainties.

p0deje commented 9 years ago

Thanks for answer and explanation!

At the moment I'm trying to find a way to properly parse results like this since it separates line number with | and also includes col:

autoload/writable_search/parser.vim|60 col 18| function! s:BuildProxies(grouped_lines)

Once I'm done with this, it should be relatively easy to implement quickfix -> writable_search without any context. I think that would already be useful and is good place to start. When it's done, I'll try to find a way to show related context.

AndrewRadev commented 9 years ago

Thankfully, there's no need to parse the quickfix window's contents directly. You can use the getqflist() function to get a list of dictionaries for every entry. See :help getqflist() for details.

p0deje commented 9 years ago

Right, thanks a lot!

p0deje commented 8 years ago

I was trying to make it work from time to time, though my vim/vimscript knowledge is not very good.

But then I've found a https://github.com/Olical/vim-enmasse plugin, which does exactly the same thing - allows to create editable buffer from quickfix. It doesn't look as great as writablesearch but does the job.

AndrewRadev commented 8 years ago

Yeah, I noticed this plugin recently as well. Good to have some alternatives :). I wish I'd worked on this feature as well, but I've also been having difficulties sitting down to get it done. If I do end up implementing it, I'll be sure to write here and close the issue.

AndrewRadev commented 8 years ago

I've finally sat down and created support for converting quickfix contents into a writable search buffer. Running :WritableSearchFromQuickfix should do the trick. Could you try it out and let me know if it seems to work for you?

I'm also not sure about the name. I could make it so that running :WritableSearch in the quickfix window does it, but it might be misleading (you need to be careful which window you're in if you want to run a brand new search). What do you think?

p0deje commented 8 years ago

Works really good! Fast and much more convenient than EnMasse so I'll stick to :WritableSearch instead.

Thanks for a great feature!

I think having a single :WritableSearch command is fine but I understand it might be misleading. I don't really have a strong preference.

AndrewRadev commented 8 years ago

Cool :). I'll leave it like it is for now, we'll see if I come up with a better idea later on in time.