Skip to content

Conversation

HiPhish
Copy link
Contributor

@HiPhish HiPhish commented Aug 16, 2016

When a new tabpage gets created it will copy the local working directory of the previous one, if there is any. Fixes #5082

This is a simple one-line fix, but there is another way to go at this: when a new tab gets allocated we could pass a reference to another tab (i.e. the current tab in our case) to the allocation function alloc_tabpage. If there is no other tab we pass NULL. Then the function can copy members from the old tab to the new tab. This is how new windows are created. However, that would mean changing the signature of the function and adjusting every call to it in the codebase. There are only two calls to it, both in src/nvim/window.c, so the change is not very invasive. What should I do?

@HiPhish HiPhish changed the title Fix new tab not inheriting local working directory. [RFC] Fix new tab not inheriting local working directory. Aug 18, 2016
@jamessan
Copy link
Member

Could you add a test for the bug you're fixing?

when a new tab gets allocated we could pass a reference to another tab (i.e. the current tab in our case) to the allocation function alloc_tabpage. If there is no other tab we pass NULL.

I think that may be an interesting refactor to look at, but this PR is fine as is to fix the immediate issue.

@marvim marvim added the RFC label Aug 18, 2016
@HiPhish
Copy link
Contributor Author

HiPhish commented Aug 18, 2016

@jamessan I have been trying to, but whenever I add some test the entire test system gets stuck indefinitely on an unrelated test (on idea which one, I only know it displays just a few green bubbles). Even if the test is something trivial like

    describe('Local directory gets inherited', function()
      it('by tabs', function()
          eq('asdf', 'asdf')
      end)
    end)

Am I missing something here?

EDIT: If I make the test fail (like changing one of the asdf to something else) then it doesn't get stuck anymore.

@justinmk
Copy link
Member

That almost always means a Press-Enter prompt is showing, though the snippet you pasted doesn't have anything that should cause that. But you could try adding helpers.wait() to make Press-Enter go away.

@HiPhish
Copy link
Contributor Author

HiPhish commented Aug 19, 2016

It feels like the test framework is held together by duct tape and chewing gum. The tests are running now, but at least on my system I couldn't find any rhyme or reason as to why it would get stuck, with or without helpers.wait() (it doesn't even use it at the moment). Let's see how the CI works out. Sometimes it was even random whether the tests would get stuck, it happened even after no changes to my test.

@justinmk
Copy link
Member

The press enter issue is ugly, and we can fix it eventually, but other than that I don't know what you mean. It's a fancy BDD framework wrapping an RPC API, offering the ability to test actual screen layout. Seems sophisticated to me :)

@HiPhish
Copy link
Contributor Author

HiPhish commented Aug 19, 2016

Maybe it's just my system, but testing seems to get randomly stuck. Maybe I was just imaging the tests to get stuck because of my changes, since I wasn't able to find any pattern to it today. I recorded a script file that shows where the tests get stuck:
http://pastebin.com/602sr1SU

This is after I pushed my commit, it's the same set of tests that has run successfully on the CI.

@justinmk
Copy link
Member

justinmk commented Aug 20, 2016

If the tests hang, change BUSTED_OUTPUT_TYPE in CMakeLists.txt to gtest instead of utfTerminal. If the next run doesn't show each individual test, rm -rf build might be required first.

That doesn't seem to work, cmake is being a pain. I had to use this:

DWORKING_DIR=/home/foo/neovim -DBUSTED_OUTPUT_TYPE=gtest -DTEST_DIR=/home/foo/neovim/test -DBUILD_DIR=/home/foo/neovim/build -DTEST_TYPE=functional -P /home/foo/neovim/cmake/RunTests.cmake

@HiPhish
Copy link
Contributor Author

HiPhish commented Aug 21, 2016

What do I do with that snippet?

When a new tabpage gets created it will copy the local working directory
of the previous one, if there is any.
@justinmk justinmk merged commit 626065d into neovim:master Sep 4, 2016
@justinmk justinmk removed the RFC label Sep 4, 2016
@justinmk
Copy link
Member

justinmk commented Sep 4, 2016

Verified test. Thanks @HiPhish

@HiPhish HiPhish deleted the fix-tcd branch September 4, 2016 07:09
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!
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.

After using tcd, tabs created after temporarily also use that working directory
4 participants