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

Fix term enabled behavior #1725

Merged
merged 3 commits into from
Mar 20, 2018
Merged

Conversation

bhcleek
Copy link
Collaborator

@bhcleek bhcleek commented Mar 20, 2018

  • Make sure that behavior is consistent when g:go_term_enabled=1 regardless of the value of the splitright option.

  • Remove the startinsert and stopinsert pair; they cancel each other out
    and are unnecessary.

  • remove superfluous stderr callback.

bhcleek added 3 commits March 20, 2018 00:25
Remove the startinsert and stopinsert pair; they cancel each other out
and are unnecessary.
Window numbers are mutable; it's only window ids that don't change.
When g:go_term_enabled=1, make sure that the window from which terminal
commands are run is made active again regardless of the value of
splitting options (e.g. 'splitright').
@codecov-io
Copy link

codecov-io commented Mar 20, 2018

Codecov Report

Merging #1725 into master will increase coverage by 0.97%.
The diff coverage is 27.27%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1725      +/-   ##
==========================================
+ Coverage   23.94%   24.91%   +0.97%     
==========================================
  Files          57       57              
  Lines        4766     4761       -5     
==========================================
+ Hits         1141     1186      +45     
+ Misses       3625     3575      -50
Flag Coverage Δ
#nvim 18.98% <27.27%> (+0.96%) ⬆️
#vim74 18.71% <0%> (+0.01%) ⬆️
#vim80 21.21% <0%> (+0.02%) ⬆️
Impacted Files Coverage Δ
autoload/go/term.vim 58.18% <27.27%> (+58.18%) ⬆️
autoload/go/tool.vim 28.47% <0%> (+9.02%) ⬆️

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 f80622f...426621f. Read the comment docs.

if state.winnr !=# winnr()
exe state.winnr . "wincmd w"
endif
call win_gotoid(state.winid)
Copy link
Owner

Choose a reason for hiding this comment

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

is this a native nvim function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is. We actually probably need to be using it many other places, but all the other cases where it should be used but isn't requires an unlikely edge case before a user would notice a problem. Because of the way the terminal works, though, we must use the window id instead of the window number to make sure the behavior is the same regardless of the value of splitright.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for the clarification.

@fatih
Copy link
Owner

fatih commented Mar 20, 2018

lgtm

@bhcleek bhcleek merged commit bb80ad5 into fatih:master Mar 20, 2018
@bhcleek bhcleek deleted the fix-term-enabled-behavior branch March 20, 2018 16:45
bhcleek added a commit that referenced this pull request Mar 20, 2018
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