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.

Sign up for GitHub

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

Jump to bottom

[RFC] Return empty string if no working directory. #5292

Closed
wants to merge 1 commit into from
Closed

[RFC] Return empty string if no working directory. #5292

wants to merge 1 commit into from

Conversation

HiPhish
Copy link
Contributor

@HiPhish HiPhish commented Sep 4, 2016

This restores behaviour identical to Vim. If the user calls the VimScript function 'getcwd()' and the working directory cannot be found (for example because the directory has been deleted since the last time it was used) an empty string needs to be returned instead of throwing an error.

I still need to get the test working properly before this PR can be considered final.

@HiPhish HiPhish mentioned this pull request Sep 4, 2016
Closed
@marvim marvim added the WIP label Sep 4, 2016
fwalch
fwalch reviewed Sep 4, 2016
View reviewed changes
test/functional/ex_cmds/cd_spec.lua

it('returns an empty string', function()
execute('cd ' .. temp_dir)
-- lfs.rmdir(temp_dir)
Copy link
Member

Choose a reason for hiding this comment

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

Adding a helpers.wait() before the rmdir works for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank's that got it working.

@HiPhish HiPhish changed the title [WIP] Return empty string if no working directory. [RFC] Return empty string if no working directory. Sep 4, 2016
@HiPhish
Copy link
Contributor Author

HiPhish commented Sep 4, 2016

OK, this should be good to go now.

@marvim marvim added RFC and removed WIP labels Sep 4, 2016
Return empty string if no working directory.
d6585e0 
This restores behaviour identical to Vim. If the user calls the
VimScript function 'getcwd()' and the working directory cannot be found
(for example because the directory has been deleted since the last time
it was used) an empty string needs to be returned instead of throwing an
error.
justinmk
justinmk reviewed Sep 4, 2016
View reviewed changes
test/functional/ex_cmds/cd_spec.lua
helpers.wait()
lfs.rmdir(temp_dir)
local working_dir = cwd()
eq('', working_dir)
Copy link
Member

Choose a reason for hiding this comment

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

If I revert the change to eval.c, this doesn't fail correctly. It seems that call() ignores VimL errors. So I changed this to helpers.eval("getcwd()").

Copy link
Contributor

Choose a reason for hiding this comment

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

I usually write such things as something like eq(0, exc_exec('let wd = getcwd()')): this makes it obvious that I check for absence of exceptions and not rely on implementation details of various helper functions.

Copy link
Member

Choose a reason for hiding this comment

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

@ZyX-I I thought about that but I think our test functions should always fail on VimL errors. Does it seem correct to you that call() would be silent?

Copy link
Contributor

Choose a reason for hiding this comment

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

Such failure is an error (BTW, why screen test failures are errors and not failures?), eq(0,…) failure is a failure which is more in line with the purpose of the test. I do not see why original variant does not fail though, nvim_call_function should generate error in this case due to the used try_* pair.

Copy link
Member

Choose a reason for hiding this comment

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

BTW, why screen test failures are errors and not failures?)

I don't think there's a reason. It's always bugged me.

justinmk pushed a commit that referenced this pull request Sep 4, 2016
@justinmk
getcwd(): Return empty string if CWD is invalid. #5292
cd321b7 
Closes #5291

Restores behaviour identical to Vim. If the user calls the VimScript
function 'getcwd()' and the working directory cannot be found (for
example because the directory has been deleted since the last time it
was used) an empty string needs to be returned instead of throwing an
error.
@justinmk
Copy link
Member

justinmk commented Sep 4, 2016

Merged

@justinmk justinmk closed this Sep 4, 2016
@justinmk justinmk removed the RFC label Sep 4, 2016
@blueyed
Copy link
Contributor

blueyed commented Sep 4, 2016

Thanks!

@HiPhish HiPhish deleted the fix-getcwd branch September 5, 2016 07:39
justinmk added a commit to justinmk/neovim that referenced this pull request Oct 27, 2016
@justinmk
NVIM v0.1.6
962e876 
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
@justinmk
NVIM v0.1.6
cc1ec95 
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
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@HiPhish @justinmk @blueyed @ZyX-I @fwalch @marvim