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

[RFC] Restore vim's tab drag behavior #4874

Closed
wants to merge 4 commits into from
Closed

Conversation

sach1t
Copy link
Contributor

@sach1t sach1t commented Jun 4, 2016

This fixes issue #4663.
It also depends on pull request #4873.

I just restored the tab drag behavior from vim. Since mouse release events now work, as of pull request #4873, the original tab drag implementation works correctly in neovim.

@marvim marvim added the RFC label Jun 4, 2016
@justinmk
Copy link
Member

justinmk commented Jun 4, 2016

Can you try to add a test? See mouse_spec.lua

@sach1t
Copy link
Contributor Author

sach1t commented Jun 4, 2016

Okay, I will add a test.

@timtadh
Copy link

timtadh commented Jun 6, 2016

I can confirm that this patch does not address the NerdTree issue I reported in #3310 (comment) .

@justinmk
Copy link
Member

justinmk commented Jun 7, 2016

Would you mind adding a commit for #3310 (comment) ? Don't need to add a test for it if it's too difficult.

@sach1t
Copy link
Contributor Author

sach1t commented Jun 7, 2016

@justinmk I tried adding the code from the comment. However, then it is no longer possible to continue dragging a tab from outside of the tab bar (as described in issue #4663) and once you go out of the tab bar it starts selecting text.

@justinmk
Copy link
Member

justinmk commented Jul 9, 2016

The ASAN build might be failing because that build is very slow. You could increase the timeout of the screen tests to 15 seconds (default is 3.5 seconds):

screen.timeout = 15000

(see tui_spec.lua)

@@ -64,6 +64,294 @@ describe('Mouse input', function()
]])
end)

describe('tab drag', function()
Copy link
Member

Choose a reason for hiding this comment

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

in this describe block:

before_each(function()
  screen.timeout = 15000
end)

@sach1t
Copy link
Contributor Author

sach1t commented Jul 9, 2016

Thanks for the help @justinmk. I thought that would do it for sure, but it seems to still be failing on the left tab drags for some of the builds. I will look over the code and tests to see if there is anything weird with the left drags and if not try a longer timeout.

@justinmk
Copy link
Member

justinmk commented Jul 9, 2016

Only 1 test failed now. I guess it needs to be 30s 😓

@sach1t
Copy link
Contributor Author

sach1t commented Jul 9, 2016

Forget 30s... I'm gonna go with 60... and then reduce :)

@sach1t
Copy link
Contributor Author

sach1t commented Jul 9, 2016

On both of the os x machines:
[ ERROR ] ...is/build/neovim/neovim/test/functional/ui/mouse_spec.lua @ 108: Mouse input tab drag in tabline to the left moves tab left
[ ERROR ] ...is/build/neovim/neovim/test/functional/ui/mouse_spec.lua @ 238: Mouse input tab drag out of tabline to the left moves tab left

... to the code and tests it is

@sach1t
Copy link
Contributor Author

sach1t commented Aug 8, 2016

It is looking a bit better. This time 1 os x build stalled, the other said the tests I added failed, and the other linux one was just having other issues.

@sach1t
Copy link
Contributor Author

sach1t commented Aug 8, 2016

Now both os x builds stalled... at least no errors this time ;)

@justinmk
Copy link
Member

@sach1t Is this ready? Can you squash it a bit and rebase?

@justinmk justinmk self-assigned this Aug 26, 2016
@sach1t
Copy link
Contributor Author

sach1t commented Aug 27, 2016

@justinmk I think the only issue was that os x did not like drags that go left. I will squash it today, rebase and see how the tests go. Hopefully it should be fine, and get merged.

@sach1t sach1t force-pushed the tab-drag branch 2 times, most recently from 2c0a97c to 08bc374 Compare August 27, 2016 18:13
@sach1t
Copy link
Contributor Author

sach1t commented Aug 27, 2016

It seems that dragging tabs left on os x comes across as a click. I am going to run some tests against master as dragging in tabline works.

@sach1t
Copy link
Contributor Author

sach1t commented Aug 27, 2016

So I brought over the tests for the in tab line dragging to master in pull request #5263. And similar to here the left dragging is failing only on os x builds. This either means it is a problem with the tests or something other than this patch. However, since the tests are passing on the other builds it probably means that it is not the tests themselves.

justinmk pushed a commit to justinmk/neovim that referenced this pull request Sep 12, 2016
justinmk added a commit to justinmk/neovim that referenced this pull request Sep 12, 2016
These tests fail on master, so it's not a regression. Changes in neovim#4874
(parent commit) seem to work (and pass most CI), so skipping these tests
is better than blocking the changes.
justinmk pushed a commit to justinmk/neovim that referenced this pull request Sep 12, 2016
justinmk added a commit to justinmk/neovim that referenced this pull request Sep 12, 2016
These tests fail on master, so it's not a regression. Changes in neovim#4874
(parent commit) seem to work (and pass most CI), so skipping these tests
is better than blocking the changes.
@justinmk
Copy link
Member

Merged in #5331. Thank you @sach1t !

@justinmk justinmk closed this Sep 12, 2016
@justinmk justinmk removed the RFC label Sep 12, 2016
justinmk added a commit that referenced this pull request Sep 12, 2016
These tests fail on master, so it's not a regression. Changes in #4874
(parent commit) seem to work (and pass most CI), so skipping these tests
is better than blocking the changes.
justinmk added a commit that referenced this pull request Sep 12, 2016
@justinmk
Copy link
Member

The left-drag test is failing (on master; twice now) in non-macOS UBSAN build (which is slow):

https://s3.amazonaws.com/archive.travis-ci.org/jobs/159554179/log.txt

so the failure is almost certainly timing-related. That means we can tweak it somehow, then enable it for macOS too.

@sach1t
Copy link
Contributor Author

sach1t commented Sep 17, 2016

@justinmk thanks. I will also look into fixing those test cases on master, so those tests can be re-enabled.

justinmk added a commit to justinmk/neovim that referenced this pull request Oct 27, 2016
FEATURES:
0b5a7e4 neovim#4432 API: external UIs can render custom popupmenu
c6ac4f8 neovim#4934 API: call any API method from vimscript
31df051 neovim#4568 API: nvim_call_atomic(): multiple calls in a single request
b268ba3 neovim#5424 API: nvim_win_get_number(), nvim_tabpage_get_number()
e7e2844 has("nvim-1.2.3") checks for a specific Nvim version
522b885 neovim#5295, neovim#5493 `:CheckHealth` checks tmux, terminfo, performance
719dae2 neovim#5384 events: allow event processing in getchar()
f25797f neovim#5386 API: metadata: Nvim version & API level
22dfe69 neovim#5389 API: metadata: "since", "deprecated_since"
605e743 Added QuickFixLine highlight group

CHANGES:
4af6ec7 neovim#5253 perf: Disable clipboard in do_cmdline()
6e9f329 neovim#5299 perf: Skip foldUpdate() in insert-mode.
9d4fcec neovim#5426 perf: Do not auto-update folds for some foldmethods.
eeec0ca neovim#5419 tui: Default to normal-mode cursor shape.

FIXES:
e838452 neovim#5436 tui: Fix "weird characters" / "bleeding termcodes"
10a54ad neovim#5243 signal_init: Always unblock SIGCHLD.
bccb49b neovim#5316 eval.c: Fix memory leak for detached pty job
626065d neovim#5227 tchdir: New tab should inherit CWD.
cd321b7 neovim#5292 getcwd(): Return empty string if CWD is invalid.
6127eae shada: Fix non-writeable ShaDa directory handling
ca65514 neovim#2789 system(): Respect shellxescape, shellxquote
2daf54e neovim#4874 Restore vim-like tab dragging
0c536b5 neovim#5319 syntax.c: Support bg/fg special color-names.
3c53371 neovim#4972 from justinmk/schedule-ui_refresh
68bcb32 neovim#4789 tui.c: Do not wait for tui loop on teardown.
c8b6ec2 neovim#5409 v:count broken in command-line window
6bc3bce neovim#5461 fix emoji display
51937e1 neovim#5470 fix :terminal with :argadd, :argu
79d77da neovim#5481 external UIs: opening multiple files from command-line
657ba62 neovim#5501 rplugin: resolve paths in manifest file
6a6f188 neovim#5502 system('foo &', 'bar'): Show error, don't crash.
1ff162c neovim#5515 os_nodetype: open fd with O_NONBLOCK
2a6c5bb neovim#5450 modeline: Handle version number overflow.
0ade1bb neovim#5225 CI tests now run against Windows!
justinmk added a commit to justinmk/neovim that referenced this pull request Oct 28, 2016
FEATURES:
0b5a7e4 neovim#4432 API: external UIs can render custom popupmenu
c6ac4f8 neovim#4934 API: call any API method from vimscript
31df051 neovim#4568 API: nvim_call_atomic(): multiple calls in a single request
b268ba3 neovim#5424 API: nvim_win_get_number(), nvim_tabpage_get_number()
e7e2844 has("nvim-1.2.3") checks for a specific Nvim version
522b885 neovim#5295, neovim#5493 `:CheckHealth` checks tmux, terminfo, performance
719dae2 neovim#5384 events: allow event processing in getchar()
f25797f neovim#5386 API: metadata: Nvim version & API level
22dfe69 neovim#5389 API: metadata: "since", "deprecated_since"
605e743 Added QuickFixLine highlight group

CHANGES:
4af6ec7 neovim#5253 perf: Disable clipboard in do_cmdline()
6e9f329 neovim#5299 perf: Skip foldUpdate() in insert-mode.
9d4fcec neovim#5426 perf: Do not auto-update folds for some foldmethods.
eeec0ca neovim#5419 tui: Default to normal-mode cursor shape.

FIXES:
e838452 neovim#5436 tui: Fix "weird characters" / "bleeding termcodes"
10a54ad neovim#5243 signal_init: Always unblock SIGCHLD.
bccb49b neovim#5316 eval.c: Fix memory leak for detached pty job
626065d neovim#5227 tchdir: New tab should inherit CWD.
cd321b7 neovim#5292 getcwd(): Return empty string if CWD is invalid.
6127eae shada: Fix non-writeable ShaDa directory handling
ca65514 neovim#2789 system(): Respect shellxescape, shellxquote
2daf54e neovim#4874 Restore vim-like tab dragging
0c536b5 neovim#5319 syntax.c: Support bg/fg special color-names.
3c53371 neovim#4972 from justinmk/schedule-ui_refresh
68bcb32 neovim#4789 tui.c: Do not wait for tui loop on teardown.
c8b6ec2 neovim#5409 v:count broken in command-line window
6bc3bce neovim#5461 fix emoji display
51937e1 neovim#5470 fix :terminal with :argadd, :argu
79d77da neovim#5481 external UIs: opening multiple files from command-line
657ba62 neovim#5501 rplugin: resolve paths in manifest file
6a6f188 neovim#5502 system('foo &', 'bar'): Show error, don't crash.
1ff162c neovim#5515 os_nodetype: open fd with O_NONBLOCK
2a6c5bb neovim#5450 modeline: Handle version number overflow.
0ade1bb neovim#5225 CI tests now run against Windows!
blueyed added a commit to blueyed/neovim that referenced this pull request Jul 23, 2019
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.

None yet

4 participants