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

Improved quickfix handling with title #44

Closed
wants to merge 2 commits into from

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Apr 9, 2016

This uses the new title argument with setqflist/setloclist, if
available to set the title with a suffix that allows to handle it via
a FileType qf autocommand later, too.

With this the qf windows will have mappings and title always (with
-noopen and after :cclose/:copen).

Fixes #40.

This uses the new `title` argument with `setqflist`/`setloclist`, if
available to set the title with a suffix that allows to handle it via
a `FileType qf` autocommand later, too.

With this the qf windows will have mappings and title always (with
`-noopen` and after `:cclose/:copen`).

Fixes mhinz#40.
@mhinz
Copy link
Owner

mhinz commented Jun 8, 2016

I like it, I just wonder if there's a way that doesn't alter w:quickfix_title. Hm..

@blueyed
Copy link
Contributor Author

blueyed commented Jun 8, 2016

Yes, it's pretty essential.

w:quickfix_title is only used as fallback, if the title argument is not supported.

@blueyed
Copy link
Contributor Author

blueyed commented Jun 8, 2016

I think in case of E118 (when it does not work), this should be remembered and not get tried again every time.

@mhinz
Copy link
Owner

mhinz commented Jun 8, 2016

w:quickfix_title is only used as fallback, if the title argument is not supported.

So it concerns all Vim users, which is probably the majority. :->

Unfortunately I also don't see any other possibility than using w:quickfix_title. Obviously we need a conceal feature for the statusline!

@mhinz
Copy link
Owner

mhinz commented Jun 8, 2016

I also noticed that Vim permanently sets the title when running :grep, so the the autocmd will never set the mappings because w:quickfix_title will be reset on :copen. Hmm...

@ches
Copy link

ches commented Jun 16, 2016

I've been through so many trains of thought in maintaining ack.vim that led me to feel that writing a new plugin almost exactly like vim-grepper was the way I wanted to go… even half-wrote something never complete enough to release before your plugin appeared (and it looks great, by the way!).

Now on this particular subject, I have pretty much made the conclusion that it just shouldn't be the search plugin's place to impose mappings on the quickfix/location list windows, at all—you can jump through hoops (kind of like part of this PR, no offense to @blueyed for trying to come up with solutions), but it boils down to the mappings sometimes being there (confusing, or frustrating), or always being there (out of scope for a search plugin to decide, if you're a purist about it).

The mappings are really a general behavior not specific to search that you end up always wanting in every list window, no matter what populated it. So I feel that QFEnter is the right idea, even if I don't totally love the implementation (but it works).

See mileszs/ack.vim#170 if you want even more of rambling musings that got me to this point.

There may be other quickfix-related features of vim-grepper you want to maintain (title) that I'm not familiar with—I'm just getting acquainted—and you may disagree about mappings or feel that it's too late to change this with users expecting it, but with hindsight I had decided to avoid going down this path in a green-field search plugin.

So, just my 2¢ here, but seriously consider not putting yourself in a no-win position with quickfix mappings.

@ches
Copy link

ches commented Jun 16, 2016

Heh, #36 is relevant.

@mhinz
Copy link
Owner

mhinz commented Jun 17, 2016

@ches You convinced me.

Technically, every possible solution for resetting mappings in the quickfix window will always be just a hack. Moreover, easy searching is the goal of this plugin, not quickfix navigation. Tilting at windmills is not what we want!

The optional {title} argument that Neovim implements for setqflist() is still useful though, so I'll change this PR's commit in that regard and remove the rest. (@blueyed Are you cool with this? I don't want to ditch the commit.)

Afterwards I'll remove the mappings and add a suggestion for either QFEnter or vim-qf. I still have to try both of them.

Thanks for your input!

@blueyed
Copy link
Contributor Author

blueyed commented Jun 17, 2016

@mhinz
Sounds good!
I assume you will just grab the code from my PR, or should I re-submit it?!

@mhinz
Copy link
Owner

mhinz commented Jun 17, 2016

@blueyed Yup, I'll do it. Just lean back and relax. :->

@mhinz mhinz closed this in 9d1f935 Jun 17, 2016
mhinz added a commit that referenced this pull request Jun 17, 2016
Navigating the quickfix list is not goal of this plugin. Use one of
these plugins instead:

    - https://github.com/romainl/vim-qf
    - https://github.com/yssl/QFEnter

References #44.
Closes #36.
@mhinz
Copy link
Owner

mhinz commented Jun 17, 2016

It's done. Thanks for code and opinions!

@blueyed
Copy link
Contributor Author

blueyed commented Jun 17, 2016

Thanks!
What about adding the "[grepper]" suffix though (https://github.com/mhinz/vim-grepper/pull/44/files#diff-e3b21649c9aa7fa4529961508f47e061R440)?

@mhinz
Copy link
Owner

mhinz commented Jun 17, 2016

If anything, I'd go for a prefix, since the tail should be the executed command itself. But why would you still need it now?

@blueyed
Copy link
Contributor Author

blueyed commented Jun 17, 2016

Prefix would be fine with me, too.
I think it's nice, because you see at a glance that it's from grepper?!

@mhinz
Copy link
Owner

mhinz commented Jun 17, 2016

I think people know which of their plugins used the quickfix list recently and I'd like to keep the title as short as possible, as the command itself can get quite long already.

@ryneeverett
Copy link

ryneeverett commented Oct 1, 2016

The optional {title} argument that Neovim implements for setqflist() is still useful though, so I'll change this PR's commit in that regard and remove the rest. (@blueyed Are you cool with this? I don't want to ditch the commit.)

Looks like the title parameter wasn't added until neovim 0.1.3, so prior versions fail with E118. Vim-grepper requiring neovim-0.1.3+ seems fine to me, but should it be documented?

@blueyed blueyed deleted the improved-qf-handling branch October 1, 2016 11:29
@blueyed
Copy link
Contributor Author

blueyed commented Oct 1, 2016

@ryneeverett
I think it should be caught, like in the original PR. But it's @mhinz's take.

FWIW: it is possible in Vim now also, but the setqflist() interface is different. It expects a dict as 3rd param.

mhinz added a commit that referenced this pull request Oct 4, 2016
mhinz added a commit that referenced this pull request Oct 4, 2016
@mhinz
Copy link
Owner

mhinz commented Oct 4, 2016

@ryneeverett 0.1.3, that's like ages ago in Neovim time. ;-) You're right, of course. I added the missing check.

@blueyed Ah, thanks for the heads-up. Now the title gets set for Vim as well.

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.

Set quickfix/location list title always
4 participants