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

fmt.vim: default to quickfix list for showing errors #1365

Merged
merged 1 commit into from
Jul 22, 2017
Merged

Conversation

fatih
Copy link
Owner

@fatih fatih commented Jul 22, 2017

Not sure why we were using location list, but I think quickfix is the
better way going forward as it's more known. The user can change the
behaviour back to locationlist if they want

let g:go_list_type = "locationlist"

Also fix fetching the correct list type setting (we were not checking it
at all)

fixes #1359

Not sure why we were using location list, but I think quickfix is the
better way going forward as it's more known. The user can change the
behaviour back to locationlist if they want

    let g:go_list_type = "locationlist"

Also fix fetching the correct list type setting (we were not checking it
at all)

fixes #1359
@fatih fatih merged commit cc28ff6 into master Jul 22, 2017
@fatih fatih deleted the fix-gofmt-list branch July 22, 2017 20:23
@bhcleek
Copy link
Collaborator

bhcleek commented Aug 11, 2017

@fatih This was the locationlist because :GoFmt only modifies the current buffer. By default, vim-go commands use the quickfix window when the command affects more than just the current buffer, and the locationlist when the command operates on the current window only. Here's what the vim docs say about the locationlist:

A location list is a window-local quickfix list. You get one after commands
like :lvimgrep, :lgrep, :lhelpgrep, :lmake, etc., which create a
location list instead of a quickfix list as the corresponding :vimgrep,
:grep, :helpgrep, :make do.
A location list is associated with a window and each window can have a
separate location list. A location list can be associated with only one
window. The location list is independent of the quickfix list.

With that in mind, do you still feel this was the right change to make?

@fatih
Copy link
Owner Author

fatih commented Aug 13, 2017

I see the value of using locationlist, but practically having a different set of shortcuts and mappings for it makes it very hard to use, even for longtime vim users as they have to remember now whether the list is a quickfix or location list and then call the command to navigate it. Hence this unification to use quickfix.

I've opened a fix though that makes it even better for not accidentally closing other quickfix window: #1407

Also opened an issue to make the settings configurable for each command: #1408 going forward this will allow anyone to use whatever they want.

@bhcleek
Copy link
Collaborator

bhcleek commented Aug 13, 2017

I like the idea of making the list type configurable for each command.

Having said that, though, the defaults for each command should still be consistent. The criteria we originally used for which window to use was to use quickfix list for errors generated from commands that affected/evaluated more than just the current buffer, and the location list for commands that evaluated only the current buffer. Making GoFmt use the quickfix list deviates from that. I wonder if there's an implication here that other commands that currently use the location list should instead be using the quickfix list. If so, what do you think the criteria should be for determining the default list type for a command?

@bhcleek
Copy link
Collaborator

bhcleek commented Aug 13, 2017

I'll cross post my last comment to #1408 since it seems a more appropriate place to continue this conversation now.

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.

:cclose does not work with :GoFmt quickfix
2 participants