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

list: make Clean() more useful #1724

Merged
merged 2 commits into from
Mar 20, 2018
Merged

list: make Clean() more useful #1724

merged 2 commits into from
Mar 20, 2018

Conversation

fatih
Copy link
Owner

@fatih fatih commented Mar 20, 2018

After cleaning a list, there is no need to show the list anymore. That's
why we call Window() to invoke the close function. Remove all
Window() calls and make Clean() more useful to avoid adding the second
line. There is no single Clean() call independently, so it makes sense
to change the meaning

After cleaning a list, there is no need to show the list anymore. That's
why we call `Window()` to invoke the close function. Remove all
`Window()` calls and make Clean() more useful to avoid adding the second
line. There is no single `Clean()` call independently, so it makes sense
to change the meaning
@bhcleek
Copy link
Collaborator

bhcleek commented Mar 20, 2018

LGTM

@codecov-io
Copy link

codecov-io commented Mar 20, 2018

Codecov Report

Merging #1724 into master will increase coverage by 0.05%.
The diff coverage is 70%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1724      +/-   ##
==========================================
+ Coverage   23.88%   23.94%   +0.05%     
==========================================
  Files          57       57              
  Lines        4773     4766       -7     
==========================================
+ Hits         1140     1141       +1     
+ Misses       3633     3625       -8
Flag Coverage Δ
#nvim 18.02% <10%> (+0.04%) ⬆️
#vim74 18.69% <70%> (+0.02%) ⬆️
#vim80 21.19% <10%> (+0.07%) ⬆️
Impacted Files Coverage Δ
autoload/go/job.vim 80.7% <ø> (+2.73%) ⬆️
autoload/go/lint.vim 65.9% <ø> (+1.1%) ⬆️
autoload/go/jobcontrol.vim 74.35% <ø> (+0.94%) ⬆️
autoload/go/test.vim 72.51% <ø> (+1.41%) ⬆️
autoload/go/rename.vim 0% <ø> (ø) ⬆️
autoload/go/term.vim 0% <ø> (ø) ⬆️
autoload/go/fmt.vim 63.33% <ø> (-0.31%) ⬇️
autoload/go/list.vim 75% <70%> (-2.78%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9463308...8d040e1. Read the comment docs.

else
cclose
endif
endif
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding this block, calling go#list#Window(a:listtype) would reduce code duplication...

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, but Window() is doing something else as well and it's get bloated. I'll add a Close() function and move out the logic there.

@fatih fatih merged commit f80622f into master Mar 20, 2018
@fatih fatih deleted the list-clear branch March 20, 2018 07:14
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.

3 participants