[vim/vim] socketserver fixes/changes (PR #19118)
Foxe Chen
- don't poll current vim instance when using serverlist()
- however list the current vim instance in serverlist() (to match X11 behaviour)
- don't make X11 feature disable socketserver feature: Not sure why I originally did this but oh well.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/19118
Commit Summary
- 3d56077 don't check current vim instance when using serverlist()
- b80af36 don't make X11 feature disable socketserver feature
File Changes
(5 files)
- M src/auto/configure (32)
- M src/config.h.in (3)
- M src/configure.ac (8)
- M src/feature.h (3)
- M src/os_unix.c (17)
Patch Links:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 1 commit.
- b827f16 fix null strcmp
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 1 commit.
- 120be8f update tests
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 1 commit.
- dab0cc0 update tests
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 1 commit.
- 5e535fb update tests
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 1 commit.
- aca93bd refactor
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 2 commits.
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 1 commit.
- cf9594a fix tests
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 1 commit.
- 9a85ec0 fix tests
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 1 commit.
- 77a96c7 fix tests
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 1 commit.
- e3ce84d fix tests
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 1 commit.
- ca2fe0b fix tests
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 1 commit.
- 889d7d9 fix tests
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 1 commit.
- 2ad4598 fix tests
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 1 commit.
- 41fea8f fix tests
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 1 commit.
- 6bf177a fix tests
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 1 commit.
- dc3efd4 fix tests
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 1 commit.
- 272dfda fix tests
You are receiving this because you are subscribed to this thread.
Foxe Chen
64-bitman left a comment (vim/vim#19118)The test test_client_server_stopinsert kept on failing on MacOS for no apparent reason so I just disabled it for Mac systems.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/testdir/test_clientserver.vim:
> @@ -229,8 +230,8 @@ func Test_client_server_stopinsert()
call remote_expr(name, 'execute("stopinsert")')
- call assert_equal('n', name->remote_expr("mode(1)"))
- call assert_equal('13', name->remote_expr("col('.')"))
+ call WaitForAssert({-> assert_equal('n', name->remote_expr("mode(1)"))})
+ cal WaitForAssert({-> assert_equal('13', name->remote_expr("col('.')"))})
⬇️ Suggested change
- cal WaitForAssert({-> assert_equal('13', name->remote_expr("col('.')"))})
+ call WaitForAssert({-> assert_equal('13', name->remote_expr("col('.')"))})
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 1 commit.
- d9678cc Update test_clientserver.vim
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#19118)Does this fix an actual bug?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#19118)BTW: I noticed that when socketserver is not available, test_vim9_builtin is completely skipped. Initially I added this in 4daf031 because that test failed for me in a remote ssh session (I suppose no server functionality was available). But can we move this test into the actual functions that use the socket server feature, so we skip only those tests that need to use remote feature?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Foxe Chen
64-bitman left a comment (vim/vim#19118)Does this fix an actual bug?
Yes it does. Before if you were to call serverlist() in a vim running as a server, there would be delay, since the vim instance would be polling for a reply from itself.
BTW: I noticed that when socketserver is not available, test_vim9_builtin is completely skipped. Initially I added this in 4daf031 because that test failed for me in a remote ssh session (I suppose no server functionality was available). But can we move this test into the actual functions that use the socket server feature, so we skip only those tests that need to use remote feature?
From what I'm getting, you want to only check for the socketserver feature in tests that need it for test_vim9_builtin? That is fine.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#19118)Before if you were to call serverlist() in a vim running as a server, there would be delay, since the vim instance would be polling for a reply from itself.
Thanks, makes sense.
On the other thing:
From what I'm getting, you want to only check for the socketserver feature in tests that need it for test_vim9_builtin? That is fine.
But is there any test actually relying on a running socketserver feature? It doesn't seem like it and commit 96b2154 only added the test to the top level of the test file, but did not add any specific tests in test_vim9_builtin.
In other words, If I comment out that test, it runs completely fine in my env, just skipping those remote tests:
Executed Test_xor() in 0.000985 seconds
Executed 487 tests in 1.040465 seconds
SKIPPED Test_balloon_show(): only works in the GUI
SKIPPED Test_browse(): browse feature missing
SKIPPED Test_debugbreak(): only works on MS-Windows
SKIPPED Test_luaeval(): lua feature missing
SKIPPED Test_mzeval(): mzscheme feature missing
SKIPPED Test_perleval(): perl feature missing
SKIPPED Test_py3eval(): python3 feature missing
SKIPPED Test_pyeval(): python feature missing
SKIPPED Test_pyxeval(): python feature missing
SKIPPED Test_remote_expr(): Environment variable DISPLAY is not set
SKIPPED Test_remote_foreground(): not pure X11 environment
SKIPPED Test_remote_peek(): Environment variable DISPLAY is not set
SKIPPED Test_remote_read(): Environment variable DISPLAY is not set
SKIPPED Test_remote_send(): Environment variable DISPLAY is not set
SKIPPED Test_remote_startserver(): Environment variable DISPLAY is not set
SKIPPED Test_rubyeval(): ruby feature missing
SKIPPED Test_server2client(): Environment variable DISPLAY is not set
SKIPPED Test_test_gui_event(): only works in the GUI
So I think we can safely remove that CheckSocketServer check at the beginning of the file? Sorry for only being slightly related, but it bothers me, that I did not notice this earlier.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 8 commits.
- 0743748 don't check current vim instance when using serverlist()
- 96311d3 don't make X11 feature disable socketserver feature
- 2141aa9 fix null strcmp
- 8b62a15 update tests
- 4552288 refactor
- 987c6aa Revert "update tests"
- 6d13ba3 add test
- 1611436 fix tests
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 1 commit.
- 2cb6a89 refactor CheckSocketServer logic
You are receiving this because you are subscribed to this thread.
Foxe Chen
64-bitman left a comment (vim/vim#19118)I have removed the CheckSocketServer check at the top. I renamed it to RunSocketServer, since that is what actually does.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Foxe Chen
@64-bitman pushed 1 commit.
- 0c6df58 refactor CheckSocketServer logic
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#19118)thanks
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#19118)there was one test failing test_usercommands because it suddenly saw the new command RunSocketServer so I had to adjust it slightly and delete the command.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.