support for customizable lists types #1415

bhcleek · 2017-08-18T06:15:31Z

Add a new setting, g:go_list_type_commands: a dictionary
whose entries allows users to customize whether to use the quickfix list
or the location list for each command. The dictionary does not require
an entry for every command; only those the user wants to specify.

When g:go_list_type is set and g:go_list_type_commands contains an entry
for a command, go:go_list_type_commands takes precedence.

I'm not totally happy with the keys for the commands in the g:go_list_type_commands dictionary. Currently the entries map very closely to the commands (e.g. :GoBuild), providing an easy to understand set of keys for most users. Unfortunately, the GoModifyTags entry applies to both :GoRemoveTags and :GoAddTags commands, because the way those two commands are implemented makes it a little difficult to vary the key used and also because it seemed strange to allow those two commands to use different lists since they are each other's inverse.

I considered using a smaller set of keys that are more closely aligned with the general class of command that needs to use the key (e.g. build, install, fmt, lint, modifytags) instead of the specific command that needs to use the key. Such a scheme would use a single key, lint, for all linting commands (e.g. :GoMetaLinter, :GoLint, :GoErrCheck)...

I'm open to suggestions on the key names and how much documentation there should be for them.

I could also use some help testing this, since there's different code paths for Vim 7, Vim 8, and Neovim.

Fixes #1408

Add a new option, g:go_list_type_commands, whose value is a dictionary whose entries allows users to customize whether to use the quickfix list or the location list for each command. The dictionary does not require an entry for every command; only those the user wants to specify. When g:go_list_type is set and g:go_list_type_commands contains an entry for a command, go:go_list_type_commands takes precedence.

arp242 · 2017-09-06T18:02:56Z

I stuck this in my vimrc:

let g:go_list_type = "locationlist"
let g:go_list_type_commands = {"GoBuild": "quickfix"}

But when using :GoBuild error still open in the location list? I would expect the quickfix list?

Am I doing it wrong, or did I discover a problem?

Using Vim 8.0.987 on Linux and vim-go master.

arp242 · 2017-09-06T18:04:19Z

I considered using a smaller set of keys that are more closely aligned with the general class of command that needs to use the key (e.g. build, install, fmt, lint, modifytags) instead of the specific command that needs to use the key. Such a scheme would use a single key, lint for all linting commands (e.g. :GoMetaLinter, :GoLint, :GoErrCheck)...

Personally I think this approach makes more sense; is there ever a situation where you want to open :GoLint output in the quickfix and :GoMetaLinter in the location list? Probably not.

That being said, I'm not "against" the current approach.

fatih · 2017-09-06T19:14:26Z

@bhcleek Why not make the for variable a list? That way it could match multiple things, so we could add ["GoAddTags", "GoRemoveTags"] for the case where it's only in one place and in other places it would be ["GoBuild"].

bhcleek · 2017-09-06T19:21:07Z

@Carpetsmoker vim-go master doesn't have this change yet, so g:go_list_type is the only variable that's being checked in your test.

bhcleek · 2017-09-06T19:22:16Z

Good idea @fatih. I'll make that change. It will simplify a few things, too.

arp242 · 2017-09-06T19:27:18Z

Sorry, I'm not sure why I said in my comment that I was using master, habit I guess (do'h). I was using this PR.

bhcleek · 2017-09-06T19:39:14Z

@fatih On second thought, I don't think it would be possible to make for a list. Imagine let g:go_list_type_commands = ["GoAddTags", "quickfix", "GoRemoveTags": "locationlist"]

What would go:list:Type(["GoAddTags", "GoRemoveTags"], "locationlist") return?

bhcleek · 2017-09-06T19:43:25Z

@Carpetsmoker looks like a bug. I know the source of it and will fix it tonight.

Fix list type fallback behavior so that it correctly handles the situation where for some command Foo, g:go_list_type's value will be respected when g:go_list_type_commands is a non-empty dictionary that does not contain an entry for Foo. Added a missing command, GoRename, to g:go_list_type_commands. When displaying the results of a command that was run in a terminal, pass "_term" to go#list#Type. _term should not be considered one of the supported values of g:go_list_type_commands.

bhcleek · 2017-09-07T05:12:12Z

@Carpetsmoker I believe I've resolved the issue you discovered. Can you verify that it's resolved for you, too?

arp242 · 2017-09-07T09:09:46Z

autoload/go/term.vim

@@ -104,7 +104,7 @@ function! s:on_exit(job_id, exit_status, event) dict abort
  endif
  let job = s:jobs[a:job_id]

-  let l:listtype = "locationlist"
+  let l:listtype = go#list#Type("_term", "locationlist")


The _term key isn't documented in the g:go_list_type_commands documentation; is that intentional?

Yes. From the most recent commit message:

When displaying the results of a command that was run in a terminal,
pass "_term" to go#list#Type. _term should not be considered one of the
supported values of g:go_list_type_commands.

arp242 · 2017-09-07T09:21:59Z

doc/vim-go.txt

+Specifies the type of list to use for command outputs (such as errors from
+builds, results from static analysis commands, etc...). The default value
+(an empty dictionary) will use the appropriate kind of list for the command
+that was called. Supported keys are "GoBuild", "GoErrCheck", "GoFmt",


I think that "The default value (an empty dictionary) will use the appropriate kind of list for the command that was called." isn't 100% true, as g:go_list_type can override it.

Perhaps it's an idea to add a single help entry for both g:go_list_type and g:go_list_type_commands which describes how vim-go figures out which list type to use? For example something along the lines of:

*'g:go_list_type'* *'g:go_list_type_commands'* Specifies the type of list to use for command outputs (such as errors from builds, results from static analysis commands, etc.) With `g:go_list_type` you can specify which list to open by default. This default value can be overridden for specific commands with `g:go_list_type_commands`, which is a dictionary to map a command to a value ("quickfix" or "locationlist"). Supported keys are "GoBuild", "GoErrCheck", "GoFmt", "GoInstall", "GoLint", "GoMetaLinter", "GoModifyTags" (used for both :GoAddTags and :GoRemoveTags), "GoRename", "GoRun", and "GoTest". If neither options are set then vim-go will use the most appropriate kind of list for the command that was called. For example to use the location list by default but the quickfix list for |:GoInstall|: > let g:go_list_type = "quickfix" let g:go_list_type_commands = {"GoBuild": "locationlist"} <

That's possible, but note that the documentation for g:go_list_type does document that it is used when g:go_list_type_commands does not contain an entry for the command:

Specifies the type of list to use for command outputs (such as errors from
builds, results from static analysis commands, etc...) when a value is not
specified for the command in g:go_list_type_commands. The default value
(empty) will use the appropriate kind of list for the command that was called.

Yeah, but if you use :help 'g:go_list_type_commands' then you won't see that part, because it's (probably) off-screen.

Your last commit looks good 👍

arp242 · 2017-09-07T09:23:33Z

Yup, seems to work well now 👍

bhcleek · 2017-09-07T14:27:19Z

I edited the doc to mention the interplay of g:go_list_type and g:go_list_type_commands.

PTAL

bhcleek · 2017-09-07T19:13:45Z

@Carpetsmoker does this need your LGTM on the PR as opposed to an in-line comment for visibility?

fatih · 2017-09-12T08:15:40Z

Thanks @bhcleek 👍

bhcleek · 2017-09-12T14:45:35Z

🙇

correct function name typos

7472d85

arp242 reviewed Sep 7, 2017

View reviewed changes

cross-reference g:go_list_type and g:go_list_type_commands

4d3a52e

arp242 approved these changes Sep 8, 2017

View reviewed changes

fatih merged commit df60f26 into fatih:master Sep 12, 2017

bhcleek deleted the customizable-lists branch September 12, 2017 14:45

bhcleek mentioned this pull request Sep 13, 2017

GoBuild command fail #1444

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support for customizable lists types #1415

support for customizable lists types #1415

bhcleek commented Aug 18, 2017 •

edited

Loading

arp242 commented Sep 6, 2017

arp242 commented Sep 6, 2017 •

edited

Loading

fatih commented Sep 6, 2017

bhcleek commented Sep 6, 2017

bhcleek commented Sep 6, 2017

arp242 commented Sep 6, 2017 •

edited

Loading

bhcleek commented Sep 6, 2017

bhcleek commented Sep 6, 2017 •

edited

Loading

bhcleek commented Sep 7, 2017

arp242 Sep 7, 2017 •

edited

Loading

bhcleek Sep 7, 2017

arp242 Sep 7, 2017

bhcleek Sep 7, 2017

arp242 Sep 7, 2017

arp242 commented Sep 7, 2017

bhcleek commented Sep 7, 2017

bhcleek commented Sep 7, 2017

fatih commented Sep 12, 2017

bhcleek commented Sep 12, 2017

support for customizable lists types #1415

support for customizable lists types #1415

Conversation

bhcleek commented Aug 18, 2017 • edited Loading

arp242 commented Sep 6, 2017

arp242 commented Sep 6, 2017 • edited Loading

fatih commented Sep 6, 2017

bhcleek commented Sep 6, 2017

bhcleek commented Sep 6, 2017

arp242 commented Sep 6, 2017 • edited Loading

bhcleek commented Sep 6, 2017

bhcleek commented Sep 6, 2017 • edited Loading

bhcleek commented Sep 7, 2017

arp242 Sep 7, 2017 • edited Loading

Choose a reason for hiding this comment

bhcleek Sep 7, 2017

Choose a reason for hiding this comment

arp242 Sep 7, 2017

Choose a reason for hiding this comment

bhcleek Sep 7, 2017

Choose a reason for hiding this comment

arp242 Sep 7, 2017

Choose a reason for hiding this comment

arp242 commented Sep 7, 2017

bhcleek commented Sep 7, 2017

bhcleek commented Sep 7, 2017

fatih commented Sep 12, 2017

bhcleek commented Sep 12, 2017

bhcleek commented Aug 18, 2017 •

edited

Loading

arp242 commented Sep 6, 2017 •

edited

Loading

arp242 commented Sep 6, 2017 •

edited

Loading

bhcleek commented Sep 6, 2017 •

edited

Loading

arp242 Sep 7, 2017 •

edited

Loading