Re: Test_exit_callback fail on Windows CUI

2016-02-25 Fir de Conversatie Bram Moolenaar 

Ken Takata wrote:

> Hi Bram,
> 
> 2016/2/25 Thu 5:56:12 UTC+9 Bram Moolenaar wrote:
> > Yukihiro Nakadaira wrote:
> > 
> > > > > On Wed, Feb 24, 2016 at 8:11 PM, mattn  wrote:
> > > > >
> > > > > > I'm thinking "kill" is not matter for this test because this test
> > > > should
> > > > > > be checking of exit.
> > > > > >
> > > > > > diff --git a/src/testdir/test_channel.vim
> > > > b/src/testdir/test_channel.vim
> > > > > > index 69922b1..e74e54c 100644
> > > > > > --- a/src/testdir/test_channel.vim
> > > > > > +++ b/src/testdir/test_channel.vim
> > > > > > @@ -483,6 +483,7 @@ func Test_exit_callback()
> > > > > >if has('job')
> > > > > >  call s:run_server('s:test_exit_callback')
> > > > > >
> > > > > > +call job_stop(s:exit_job, "kill")
> > > > > >  " the job may take a little while to exit
> > > > > >  sleep 50m
> > > > > >
> > > > >
> > > > > In CUI Vim, job_stop(job, "hup") doesn't work because AttachConsole()
> > > > fails.
> > > > > The following patch might fix it.  Job process is created in same
> > > > console.
> > > > > I'm not sure if it doesn't causes another problem.
> > > >
> > > > There is a todo item to make it possible to start a terminal for the job
> > > > to run in.  This is especially useful if the job produces some output
> > > > and perhaps even prompts for input.  We don't want that to get mixed up
> > > > with the Vim display.
> > > >
> > > > So hopefully we can make both work.  This would require the job to be
> > > > started with a socket, so its stdin/stdout go to the terminal.
> > > >
> > > 
> > > I see.  So "kill" is a simple solution.
> > 
> > I'm wondering, if this test fails because the job didn't finish yet,
> > don't we have the same problem with every other test, since they all
> > start the server?
> 
> Yes, you are right.
> When running test_channel.vim on Win32 CUI, many python processes are left.
> 
> > It seems that the default behavior for mch_stop_job() is being too
> > gentle.  And it only has "kill" and everything else.  And that means
> > sending CTRL_C_EVENT or CTRL_BREAK_EVENT.
> > 
> > Perhaps we should make the default behave like "kill" and only have
> > "hup" and "int" send an event?  The default is actually supposed to be
> > the same as "term", which for MS-Windows probably is the same as "kill".
> 
> I wrote a patch to change the default to "kill" on Windows.  Other
> behaviors are not changed.  With this patch, no python processes are
> left, and of cause the test_channel.vim passes.

Thanks.  I'll make the MS-Windows and Unix implementations accept the
same arguments, and do what is most similar.  For both the default will
be "term", which should stop the job normally.  For MS-Windows that
means killing it, since there is no "gentle" way.

-- 
BEDEVERE: Look!  It's the old man from scene 24 - what's he Doing here?
ARTHUR:   He is the keeper of the Bridge.  He asks each traveler five
  questions ...
GALAHAD:  Three questions.
 "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

 /// Bram Moolenaar -- b...@moolenaar.net -- http://www.Moolenaar.net   \\\
///sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\  an exciting new programming language -- http://www.Zimbu.org///
 \\\help me help AIDS victims -- http://ICCF-Holland.org///

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Test_exit_callback fail on Windows CUI

2016-02-25 Fir de Conversatie Ken Takata 
Hi Bram,

2016/2/25 Thu 5:56:12 UTC+9 Bram Moolenaar wrote:
> Yukihiro Nakadaira wrote:
> 
> > > > On Wed, Feb 24, 2016 at 8:11 PM, mattn  wrote:
> > > >
> > > > > I'm thinking "kill" is not matter for this test because this test
> > > should
> > > > > be checking of exit.
> > > > >
> > > > > diff --git a/src/testdir/test_channel.vim
> > > b/src/testdir/test_channel.vim
> > > > > index 69922b1..e74e54c 100644
> > > > > --- a/src/testdir/test_channel.vim
> > > > > +++ b/src/testdir/test_channel.vim
> > > > > @@ -483,6 +483,7 @@ func Test_exit_callback()
> > > > >if has('job')
> > > > >  call s:run_server('s:test_exit_callback')
> > > > >
> > > > > +call job_stop(s:exit_job, "kill")
> > > > >  " the job may take a little while to exit
> > > > >  sleep 50m
> > > > >
> > > >
> > > > In CUI Vim, job_stop(job, "hup") doesn't work because AttachConsole()
> > > fails.
> > > > The following patch might fix it.  Job process is created in same
> > > console.
> > > > I'm not sure if it doesn't causes another problem.
> > >
> > > There is a todo item to make it possible to start a terminal for the job
> > > to run in.  This is especially useful if the job produces some output
> > > and perhaps even prompts for input.  We don't want that to get mixed up
> > > with the Vim display.
> > >
> > > So hopefully we can make both work.  This would require the job to be
> > > started with a socket, so its stdin/stdout go to the terminal.
> > >
> > 
> > I see.  So "kill" is a simple solution.
> 
> I'm wondering, if this test fails because the job didn't finish yet,
> don't we have the same problem with every other test, since they all
> start the server?

Yes, you are right.
When running test_channel.vim on Win32 CUI, many python processes are left.


> It seems that the default behavior for mch_stop_job() is being too
> gentle.  And it only has "kill" and everything else.  And that means
> sending CTRL_C_EVENT or CTRL_BREAK_EVENT.
> 
> Perhaps we should make the default behave like "kill" and only have
> "hup" and "int" send an event?  The default is actually supposed to be
> the same as "term", which for MS-Windows probably is the same as "kill".

I wrote a patch to change the default to "kill" on Windows.  Other behaviors are
not changed.  With this patch, no python processes are left, and of cause
the test_channel.vim passes.

Regards,
Ken Takata 

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
# HG changeset patch
# Parent  34e55cf88be6d4dfff3f45f7d702b58afc2957d4

diff --git a/runtime/doc/eval.txt b/runtime/doc/eval.txt
--- a/runtime/doc/eval.txt
+++ b/runtime/doc/eval.txt
@@ -4474,21 +4474,22 @@ job_status({job})	*job_status()* *E9
 job_stop({job} [, {how}])	*job_stop()*
 		Stop the {job}.  This can also be used to signal the job.
 
-		When {how} is omitted or is "term" the job will be terminated
-		normally.  For Unix SIGTERM is sent.  For MS-Windows
-		CTRL_BREAK will be sent.  This goes to the process group, thus
-		children may also be affected.
+		When {how} is omitted the job will be terminated normally on
+		Unix.  On MS-Windows the job will be terminated forcedly.
+		This goes to the process group, thus children may also be
+		affected.
 
 		Other values for Unix:
-			"hup"	Unix: SIGHUP
-			"quit"	Unix: SIGQUIT
-			"kill"	Unix: SIGKILL (strongest way to stop)
-			number	Unix: signal with that number
+			"term"  SIGTERM (default)
+			"hup"	SIGHUP
+			"quit"	SIGQUIT
+			"kill"	SIGKILL (strongest way to stop)
+			number	signal with that number
 
 		Other values for MS-Windows:
-			"int"	Windows: CTRL_C
-			"kill"	Windows: terminate process forcedly
-			Others	Windows: CTRL_BREAK
+			"kill"	terminate process forcedly (default)
+			"int"	CTRL_C
+			Others	CTRL_BREAK
 
 		On Unix the signal is sent to the process group.  This means
 		that when the job is "sh -c command" it affects both the shell
diff --git a/src/os_win32.c b/src/os_win32.c
--- a/src/os_win32.c
+++ b/src/os_win32.c
@@ -5149,7 +5149,7 @@ mch_stop_job(job_T *job, char_u *how)
 int ret = 0;
 int ctrl_c = STRCMP(how, "int") == 0;
 
-if (STRCMP(how, "kill") == 0)
+if (STRCMP(how, "kill") == 0 || *how == NUL)
 {
 	if (job->jv_job_object != NULL)
 	return TerminateJobObject(job->jv_job_object, 0) ? OK : FAIL;

Re: Test_exit_callback fail on Windows CUI

2016-02-25 Fir de Conversatie Yukihiro Nakadaira 
On Thu, Feb 25, 2016 at 5:56 AM, Bram Moolenaar  wrote:

>
> Yukihiro Nakadaira wrote:
>
> > > > On Wed, Feb 24, 2016 at 8:11 PM, mattn  wrote:
> > > >
> > > > > I'm thinking "kill" is not matter for this test because this test
> > > should
> > > > > be checking of exit.
> > > > >
> > > > > diff --git a/src/testdir/test_channel.vim
> > > b/src/testdir/test_channel.vim
> > > > > index 69922b1..e74e54c 100644
> > > > > --- a/src/testdir/test_channel.vim
> > > > > +++ b/src/testdir/test_channel.vim
> > > > > @@ -483,6 +483,7 @@ func Test_exit_callback()
> > > > >if has('job')
> > > > >  call s:run_server('s:test_exit_callback')
> > > > >
> > > > > +call job_stop(s:exit_job, "kill")
> > > > >  " the job may take a little while to exit
> > > > >  sleep 50m
> > > > >
> > > >
> > > > In CUI Vim, job_stop(job, "hup") doesn't work because AttachConsole()
> > > fails.
> > > > The following patch might fix it.  Job process is created in same
> > > console.
> > > > I'm not sure if it doesn't causes another problem.
> > >
> > > There is a todo item to make it possible to start a terminal for the
> job
> > > to run in.  This is especially useful if the job produces some output
> > > and perhaps even prompts for input.  We don't want that to get mixed up
> > > with the Vim display.
> > >
> > > So hopefully we can make both work.  This would require the job to be
> > > started with a socket, so its stdin/stdout go to the terminal.
> > >
> >
> > I see.  So "kill" is a simple solution.
>
> I'm wondering, if this test fails because the job didn't finish yet,
> don't we have the same problem with every other test, since they all
> start the server?
>
> It seems that the default behavior for mch_stop_job() is being too
> gentle.  And it only has "kill" and everything else.  And that means
> sending CTRL_C_EVENT or CTRL_BREAK_EVENT.
>
> Perhaps we should make the default behave like "kill" and only have
> "hup" and "int" send an event?  The default is actually supposed to be
> the same as "term", which for MS-Windows probably is the same as "kill".
>

I agree with you to make "kill" default.

-- 
Yukihiro Nakadaira - yukihiro.nakada...@gmail.com

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Test_exit_callback fail on Windows CUI

2016-02-24 Fir de Conversatie mattn 
On Thursday, February 25, 2016 at 5:56:12 AM UTC+9, Bram Moolenaar wrote:
> Yukihiro Nakadaira wrote:
> > I see.  So "kill" is a simple solution.
> 
> I'm wondering, if this test fails because the job didn't finish yet,
> don't we have the same problem with every other test, since they all
> start the server?
> 
> It seems that the default behavior for mch_stop_job() is being too
> gentle.  And it only has "kill" and everything else.  And that means
> sending CTRL_C_EVENT or CTRL_BREAK_EVENT.

I prefer to include nakadaira's patch and set kill as default.

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Test_exit_callback fail on Windows CUI

2016-02-24 Fir de Conversatie Bram Moolenaar 

Yukihiro Nakadaira wrote:

> > > On Wed, Feb 24, 2016 at 8:11 PM, mattn  wrote:
> > >
> > > > I'm thinking "kill" is not matter for this test because this test
> > should
> > > > be checking of exit.
> > > >
> > > > diff --git a/src/testdir/test_channel.vim
> > b/src/testdir/test_channel.vim
> > > > index 69922b1..e74e54c 100644
> > > > --- a/src/testdir/test_channel.vim
> > > > +++ b/src/testdir/test_channel.vim
> > > > @@ -483,6 +483,7 @@ func Test_exit_callback()
> > > >if has('job')
> > > >  call s:run_server('s:test_exit_callback')
> > > >
> > > > +call job_stop(s:exit_job, "kill")
> > > >  " the job may take a little while to exit
> > > >  sleep 50m
> > > >
> > >
> > > In CUI Vim, job_stop(job, "hup") doesn't work because AttachConsole()
> > fails.
> > > The following patch might fix it.  Job process is created in same
> > console.
> > > I'm not sure if it doesn't causes another problem.
> >
> > There is a todo item to make it possible to start a terminal for the job
> > to run in.  This is especially useful if the job produces some output
> > and perhaps even prompts for input.  We don't want that to get mixed up
> > with the Vim display.
> >
> > So hopefully we can make both work.  This would require the job to be
> > started with a socket, so its stdin/stdout go to the terminal.
> >
> 
> I see.  So "kill" is a simple solution.

I'm wondering, if this test fails because the job didn't finish yet,
don't we have the same problem with every other test, since they all
start the server?

It seems that the default behavior for mch_stop_job() is being too
gentle.  And it only has "kill" and everything else.  And that means
sending CTRL_C_EVENT or CTRL_BREAK_EVENT.

Perhaps we should make the default behave like "kill" and only have
"hup" and "int" send an event?  The default is actually supposed to be
the same as "term", which for MS-Windows probably is the same as "kill".

-- 
BROTHER MAYNARD: Armaments Chapter Two Verses Nine to Twenty One.
ANOTHER MONK:And St.  Attila raised his hand grenade up on high saying "O
 Lord bless this thy hand grenade that with it thou mayest
 blow thine enemies to tiny bits, in thy mercy. "and the Lord
 did grin and people did feast upon the lambs and sloths and
 carp and anchovies and orang-utans and breakfast cereals and
 fruit bats and...
BROTHER MAYNARD: Skip a bit brother ...
 "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

 /// Bram Moolenaar -- b...@moolenaar.net -- http://www.Moolenaar.net   \\\
///sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\  an exciting new programming language -- http://www.Zimbu.org///
 \\\help me help AIDS victims -- http://ICCF-Holland.org///

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Test_exit_callback fail on Windows CUI

2016-02-24 Fir de Conversatie Yukihiro Nakadaira 
On Wed, Feb 24, 2016 at 11:54 PM, Bram Moolenaar  wrote:

>
> Yukihiro Nakadaira wrote:
>
> > On Wed, Feb 24, 2016 at 8:11 PM, mattn  wrote:
> >
> > > I'm thinking "kill" is not matter for this test because this test
> should
> > > be checking of exit.
> > >
> > > diff --git a/src/testdir/test_channel.vim
> b/src/testdir/test_channel.vim
> > > index 69922b1..e74e54c 100644
> > > --- a/src/testdir/test_channel.vim
> > > +++ b/src/testdir/test_channel.vim
> > > @@ -483,6 +483,7 @@ func Test_exit_callback()
> > >if has('job')
> > >  call s:run_server('s:test_exit_callback')
> > >
> > > +call job_stop(s:exit_job, "kill")
> > >  " the job may take a little while to exit
> > >  sleep 50m
> > >
> >
> > In CUI Vim, job_stop(job, "hup") doesn't work because AttachConsole()
> fails.
> > The following patch might fix it.  Job process is created in same
> console.
> > I'm not sure if it doesn't causes another problem.
>
> There is a todo item to make it possible to start a terminal for the job
> to run in.  This is especially useful if the job produces some output
> and perhaps even prompts for input.  We don't want that to get mixed up
> with the Vim display.
>
> So hopefully we can make both work.  This would require the job to be
> started with a socket, so its stdin/stdout go to the terminal.
>

I see.  So "kill" is a simple solution.

-- 
Yukihiro Nakadaira - yukihiro.nakada...@gmail.com

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Test_exit_callback fail on Windows CUI

2016-02-24 Fir de Conversatie Bram Moolenaar 

Yukihiro Nakadaira wrote:

> On Wed, Feb 24, 2016 at 8:11 PM, mattn  wrote:
> 
> > I'm thinking "kill" is not matter for this test because this test should
> > be checking of exit.
> >
> > diff --git a/src/testdir/test_channel.vim b/src/testdir/test_channel.vim
> > index 69922b1..e74e54c 100644
> > --- a/src/testdir/test_channel.vim
> > +++ b/src/testdir/test_channel.vim
> > @@ -483,6 +483,7 @@ func Test_exit_callback()
> >if has('job')
> >  call s:run_server('s:test_exit_callback')
> >
> > +call job_stop(s:exit_job, "kill")
> >  " the job may take a little while to exit
> >  sleep 50m
> >
> 
> In CUI Vim, job_stop(job, "hup") doesn't work because AttachConsole() fails.
> The following patch might fix it.  Job process is created in same console.
> I'm not sure if it doesn't causes another problem.

There is a todo item to make it possible to start a terminal for the job
to run in.  This is especially useful if the job produces some output
and perhaps even prompts for input.  We don't want that to get mixed up
with the Vim display.

So hopefully we can make both work.  This would require the job to be
started with a socket, so its stdin/stdout go to the terminal.


-- 
Why isn't there mouse-flavored cat food?

 /// Bram Moolenaar -- b...@moolenaar.net -- http://www.Moolenaar.net   \\\
///sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\  an exciting new programming language -- http://www.Zimbu.org///
 \\\help me help AIDS victims -- http://ICCF-Holland.org///

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Test_exit_callback fail on Windows CUI

2016-02-24 Fir de Conversatie Yukihiro Nakadaira 
On Wed, Feb 24, 2016 at 8:11 PM, mattn  wrote:

> I'm thinking "kill" is not matter for this test because this test should
> be checking of exit.
>
> diff --git a/src/testdir/test_channel.vim b/src/testdir/test_channel.vim
> index 69922b1..e74e54c 100644
> --- a/src/testdir/test_channel.vim
> +++ b/src/testdir/test_channel.vim
> @@ -483,6 +483,7 @@ func Test_exit_callback()
>if has('job')
>  call s:run_server('s:test_exit_callback')
>
> +call job_stop(s:exit_job, "kill")
>  " the job may take a little while to exit
>  sleep 50m
>

In CUI Vim, job_stop(job, "hup") doesn't work because AttachConsole() fails.
The following patch might fix it.  Job process is created in same console.
I'm
not sure if it doesn't causes another problem.

diff --git a/src/os_win32.c b/src/os_win32.c
index e93e6d0..5c45ff2 100644
--- a/src/os_win32.c
+++ b/src/os_win32.c
@@ -5071,8 +5071,7 @@ mch_start_job(char *cmd, job_T *job, jobopt_T
*options)
 if (!vim_create_process(cmd, TRUE,
  CREATE_SUSPENDED |
  CREATE_DEFAULT_ERROR_MODE |
- CREATE_NEW_PROCESS_GROUP |
- CREATE_NEW_CONSOLE,
+ CREATE_NEW_PROCESS_GROUP,
  , ))
 {
  CloseHandle(jo);
@@ -5152,13 +5151,17 @@ mch_stop_job(job_T *job, char_u *how)
  return TerminateProcess(job->jv_proc_info.hProcess, 0) ? OK : FAIL;
 }

+#ifdef FEAT_GUI_W32
 if (!AttachConsole(job->jv_proc_info.dwProcessId))
  return FAIL;
+#endif
 ret = GenerateConsoleCtrlEvent(
  ctrl_c ? CTRL_C_EVENT : CTRL_BREAK_EVENT,
  job->jv_proc_info.dwProcessId)
  ? OK : FAIL;
+#ifdef FEAT_GUI_W32
 FreeConsole();
+#endif
 return ret;
 }


-- 
Yukihiro Nakadaira - yukihiro.nakada...@gmail.com

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.