Fix #2524: Use own interval to call AutoTypeInfo/AutoSameids #2529

delphinus · 2019-10-01T10:53:01Z

See #2524

now developing

TODO

Deal with vim74 that has no timer support?

bhcleek · 2019-10-01T17:00:22Z

no need to worry about Vim 7.4. vim-go no longer supports it.

bhcleek · 2019-10-02T19:40:30Z

Thank you for contributing. This looks great.

Are you waiting for something specific before promoting it from a draft?

delphinus · 2019-10-03T03:01:25Z

@bhcleek

Nope, sorry for late response. I have been considering one problem.

This patch makes it start on CursorMove event. This occurs very frequently, so it calls go#auto#cursor_moved() many times. This func is lightweight and almost users do not notice any delay. But it should not be called at all when users have disabled AutoTypeInfo/AutoSameids.

So I am planning to implement with a strategy below.

Set autocmd CursorMove in plugin/go.vim only when g:go_auto_type_info == 1 or g:go_auto_sameids == 1.
Set / unset the autocmd when either :GoAutoTypeInfoToggle or :GoSameIdsAutoToggle is called.

Perhaps this works good, but introduces breaking changes.

Before: It toggles the features immediately when either value of g:go_auto_type_info / g:go_auto_sameids is changed.
After: It does nothing when either value of g:go_auto_type_info / g:go_auto_sameids is changed. Users must call :GoAutoTypeInfoToggle / :GoSameIdsAutoToggle to toggle the features.

I think users rarely change the values for the purpose of toggling the features, so this is a permissiable change. How do you think?

bhcleek · 2019-10-03T21:49:51Z

The performance of this PR will definitely be key to getting it merged.

Instead of only checking at startup time, perhaps use CursorHold to check g:go_auto_type_info and g:go_auto_sameids and hook up the autocmd if it was previously off and the variables are now set.

delphinus · 2019-10-04T01:00:08Z

It seems good! OK, I try to implement with both CursorHoold and CursorMove events.

CursorMoved events are not called after the toggling commands are input. So this commit enables to call them explicitly.

delphinus · 2019-10-04T10:22:29Z

Finished the logic. All that is left is to test.

bhcleek · 2019-10-04T19:56:13Z

autoload/go/auto.vim

-    return
-  endif
+function! go#auto#update_autocmd()
+  execute 'augroup vim-go-buffer-auto-' . bufnr('')


What's the value of using a group that's unique to the buffer? Will augroup vim-go-buffer-auto suffice?

That's right. I tested and recognized vim-go-suffer-auto is just enough. Fixed on bb1d460

bhcleek · 2019-10-04T20:03:33Z

ftplugin/go.vim

-  let &l:updatetime= get(g:, "go_updatetime", 800)
-endif
+" Set up autocmd to call :GoSameIds / :GoInfo automatically
+call go#auto#update_autocmd()


Given that there's a CursorHold autocmd that calls go#auto#update_autocmd, is this call necessary?

Not exactly because BufEnter does. I removed this line: 3b576be.

autoload/go/auto.vim

ref https://github.com/fatih/vim-go/pull/2529/files#r331662828

This reverts commit ddb96ce.

See fatih#2529 (comment)

Because update_autocmd() does it

bhcleek · 2019-10-06T20:43:06Z

This looks great. Thank you for the contribution!

I'll play with this for a couple of weeks to make sure it's working as we expect before merging it.

bhcleek · 2019-10-19T23:28:11Z

@delphinus this has been working great on my machine for a while now, and I'm planning to merge it soon. I'm taking a moment to step back, though, and ask if we really need it. Playing devil's advocate: using updatetime and CursorHold is sufficient, while this makes the code more complicated. What's the argument against using updatetime as you see it?

bhcleek · 2019-10-20T00:40:27Z

The reason to not use updatetime was laid out pretty clearly in #1055. I'm going to merge this.

bhcleek · 2019-10-20T00:40:39Z

Thank for all the hard work @delphinus

delphinus · 2019-10-20T14:10:23Z

Yeah, I noticed this problem with using airblade/vim-gitgutter, that recommends set updatetime=100 to refresh the gutters more frequently. To update AutoTypeInfo/AutoSameids less frequently than the gutters, this feature is quite useful for me, at least. ;)

v1.22 - (January 30, 2020) BACKWARDS INCOMPATIBILITIES: * Drop support for Vim 7.4. The minimum required version of Vim is now 8.0.1453. [[GH-2495]](fatih/vim-go#2495) [[GH-2497]](fatih/vim-go#2497) * Drop support for `gometalinter` [[GH-2494]](fatih/vim-go#2494) IMPROVEMENTS: * Highlight the `go` keyword in go.mod files. [[GH-2473]](fatih/vim-go#2473) * Use echo functions consistently. [[GH-2458]](fatih/vim-go#2458) * Add support for managing goroutines in debugger. [[GH-2463]](fatih/vim-go#2463) [[GH-2527]](fatih/vim-go#2527) * Document `g:go_doc_popup_window`. [[GH-2506]](fatih/vim-go#2506) * Make `g:go_doc_popup_window=1` work for Neovim, too. [[GH-2451]](fatih/vim-go#2451) [[GH-2512]](fatih/vim-go#2512) * Handle errors jumping to a definition in a file open in another Vim process better. [[GH-2518]](fatih/vim-go#2518) * Improve the UX when the gopls binary is missing. [[GH-2522]](fatih/vim-go#2522) * Use gopls instead of guru for `:GoSameIds`. [[GH-2519]](fatih/vim-go#2519) * Use gopls instead of guru for `:GoReferrers`. [[GH-2535]](fatih/vim-go#2535) * Update documentation for `g:go_addtags_transform`. [[GH-2541]](fatih/vim-go#2541) * Install most helper tools in module aware mode. [[GH-2545]](fatih/vim-go#2545) * Add a new option, `g:go_referrers_mode` to allow the user to choose whether to use gopls or guru for finding references. [[GH-2566]](fatih/vim-go#2566) * Add options to control how gopls responds to completion requests. [[GH-2567]](fatih/vim-go#2567) [[GH-2568]](fatih/vim-go#2568) * Add syntax highlighting for binary literals. [[GH-2557]](fatih/vim-go#2557) * Improve highlighting of invalid numeric literals. [[GH-2571]](fatih/vim-go#2571) [[GH-2587]](fatih/vim-go#2587) [[GH-2589]](fatih/vim-go#2589) [[GH-2584]](fatih/vim-go#2584) [[GH-2597]](fatih/vim-go#2597) [[GH-2599]](fatih/vim-go#2599) * Add highlighting of sections reported by gopls diagnostics' errors and warnings. [[GH-2569]](fatih/vim-go#2569) [[GH-2643]](fatih/vim-go#2643) * Make the highlighting of fzf decls configurable. [[GH-2572]](fatih/vim-go#2572) [[GH-2579]](fatih/vim-go#2579) * Support renaming with gopls. [[GH-2577]](fatih/vim-go#2577) [[GH-2618]](fatih/vim-go#2618) * Add an option, `g:go_gopls_enabled`, to allow gopls integration to be disabled. [[GH-2605]](fatih/vim-go#2605) [[GH-2609]](fatih/vim-go#2609) [[GH-2638]](fatih/vim-go#2638) [[GH-2640]](fatih/vim-go#2640) * Add a buffer level option, `b:go_fmt_options`, to control formatting options per buffer. [[GH-2613]](fatih/vim-go#2613) * Use build tags when running `:GoVet`. [[GH-2615]](fatih/vim-go#2615) * Add new snippets for UltiSnips. [[GH-2623]](fatih/vim-go#2623) [[GH-2627]](fatih/vim-go#2627) * Expand completions as snippets when `g:go_gopls_use_placeholders` is set. [[GH-2624]](fatih/vim-go#2624) * Add a new function, `:GoDiagnostics` and an associated mapping for seeing `gopls` diagnostics. Because of the performance implications on large projects, `g:go_diagnostics_enabled` controls whether all diagnostics are processed or only the diagnostics for the current buffer. [[GH-2612]](fatih/vim-go#2612) * Explain how to find and detect multiple copies of vim-go in the FAQ. [[GH-2632]](fatih/vim-go#2632) * Update the issue template to ask for the gopls version and `:GoReportGitHubIssue` to provide it. [[GH-2630]](fatih/vim-go#2630) * Use text properties when possible for some highlighting cases. [[GH-2652]](fatih/vim-go#2652) [[GH-2662]](fatih/vim-go#2662) [[GH-2663]](fatih/vim-go#2663) [[GH-2672]](fatih/vim-go#2672) [[GH-2678]](fatih/vim-go#2678) BUG FIXES: * Fix removal of missing directories from gopls workspaces. [[GH-2507]](fatih/vim-go#2507) * Change to original window before trying to change directories when term job ends. [[GH-2508]](fatih/vim-go#2508) * Swallow errors when the hover info cannot be determined. [[GH-2515]](fatih/vim-go#2515) * Fix errors when trying to debug lsp and hover. [[GH-2516]](fatih/vim-go#2516) * Reset environment variables on Vim <= 8.0.1831 . [[GH-2523]](fatih/vim-go#2523) * Handle empty results from delve. [[GH-2526]](fatih/vim-go#2526) * Do not overwrite `updatetime` when `g:go_auto_sameids` or `g:go_auto_type_info` is set. [[GH-2529]](fatih/vim-go#2529) * Fix example for `g:go_debug_log_output` in docs. [[GH-2547]](fatih/vim-go#2547) * Use FileChangedShellPost instead of FileChangedShell so that reload messages are not hidden. [[GH-2549]](fatih/vim-go#2549) * Restore cwd after `:GoTest` when `g:go_term_enabled` is set. [[GH-2556]](fatih/vim-go#2556) * Expand struct variable correctly in the variables debug window. [[GH-2574]](fatih/vim-go#2574) * Show output from errcheck when there are failures other than problems it can report. [[GH-2667]](fatih/vim-go#2667) Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>

Use own interval to call AutoTypeInfo/AutoSameids

d064d59

Use united config function to get updatetime

c33809c

delphinus added 4 commits October 4, 2019 18:30

Define autocmd's on demand

02eb439

Use buffer-specific timer_id

ddb96ce

Start immediately when toggle commands is called

8d7474c

CursorMoved events are not called after the toggling commands are input. So this commit enables to call them explicitly.

Start in entering another buffer

987fdae

delphinus added 2 commits October 4, 2019 20:04

Fix warnings from vimlint

3e0fedd

Disable warnings from vint

93ac77b

bhcleek reviewed Oct 4, 2019

View reviewed changes

delphinus marked this pull request as ready for review October 5, 2019 05:17

delphinus added 5 commits October 5, 2019 14:52

Use the same augroup name

bb1d460

ref https://github.com/fatih/vim-go/pull/2529/files#r331662828

Do not call here because BufEnter events does

3b576be

Revert "Use buffer-specific timer_id"

1b6a7dd

This reverts commit ddb96ce.

Define augroup's effectively

1a5ff26

See fatih#2529 (comment)

Do not restart timer redundantly

4f1d11d

Because update_autocmd() does it

delphinus added 3 commits October 7, 2019 18:37

Use &updatetime when g:go_updatetime == 0

d9bfec3

Update doc

7baedaa

Fix errors reported by vimhelplint

8082ef3

bhcleek merged commit e65fc4e into fatih:master Oct 20, 2019

bhcleek added a commit that referenced this pull request Oct 20, 2019

update CHANGELOG.md for #2529

2aa7c9d

delphinus deleted the feature/use-own-interval branch October 20, 2019 14:10

bhcleek added this to the vim-go 1.22 milestone Nov 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #2524: Use own interval to call AutoTypeInfo/AutoSameids #2529

Fix #2524: Use own interval to call AutoTypeInfo/AutoSameids #2529

delphinus commented Oct 1, 2019 •

edited

Loading

bhcleek commented Oct 1, 2019

bhcleek commented Oct 2, 2019

delphinus commented Oct 3, 2019

bhcleek commented Oct 3, 2019

delphinus commented Oct 4, 2019

delphinus commented Oct 4, 2019

bhcleek Oct 4, 2019

delphinus Oct 5, 2019

bhcleek Oct 4, 2019

delphinus Oct 5, 2019

bhcleek commented Oct 6, 2019 •

edited

Loading

bhcleek commented Oct 19, 2019

bhcleek commented Oct 20, 2019

bhcleek commented Oct 20, 2019

delphinus commented Oct 20, 2019

Fix #2524: Use own interval to call AutoTypeInfo/AutoSameids #2529

Fix #2524: Use own interval to call AutoTypeInfo/AutoSameids #2529

Conversation

delphinus commented Oct 1, 2019 • edited Loading

TODO

bhcleek commented Oct 1, 2019

bhcleek commented Oct 2, 2019

delphinus commented Oct 3, 2019

bhcleek commented Oct 3, 2019

delphinus commented Oct 4, 2019

delphinus commented Oct 4, 2019

bhcleek Oct 4, 2019

Choose a reason for hiding this comment

delphinus Oct 5, 2019

Choose a reason for hiding this comment

bhcleek Oct 4, 2019

Choose a reason for hiding this comment

delphinus Oct 5, 2019

Choose a reason for hiding this comment

bhcleek commented Oct 6, 2019 • edited Loading

bhcleek commented Oct 19, 2019

bhcleek commented Oct 20, 2019

bhcleek commented Oct 20, 2019

delphinus commented Oct 20, 2019

delphinus commented Oct 1, 2019 •

edited

Loading

bhcleek commented Oct 6, 2019 •

edited

Loading