patch 7.4.1356

Problem:    Job and channel options parsing is scattered.
Solution:   Move all option value parsing to get_job_options();

src/channel.c

@@ -748,24 +748,6 @@ channel_set_job(channel_T *channel, job_T *job)
     channel->ch_job = job;
 }
 
-/*
- * Set the mode of channel "channel" to "mode".
- */
-    void
-channel_set_mode(channel_T *channel, ch_mode_T mode)
-{
-    channel->ch_mode = mode;
-}
-
-/*
- * Set the read timeout of channel "channel".
- */
-    void
-channel_set_timeout(channel_T *channel, int timeout)
-{
-    channel->ch_timeout = timeout;
-}
-
 /*
  * Set the callback for channel "channel".
  */
@@ -782,9 +764,13 @@ channel_set_callback(channel_T *channel, char_u *callback)
     void
 channel_set_options(channel_T *channel, jobopt_T *options)
 {
-    channel_set_mode(channel, options->jo_mode);
+    if (options->jo_set & JO_MODE)
+	channel->ch_mode = options->jo_mode;
+    if (options->jo_set & JO_TIMEOUT)
+	channel->ch_timeout = options->jo_timeout;
 
-    if (options->jo_callback != NULL && *options->jo_callback != NUL)
+    if ((options->jo_set & JO_CALLBACK)
+	    && options->jo_callback != NULL && *options->jo_callback != NUL)
 	channel_set_callback(channel, options->jo_callback);
 }
 

src/eval.c

@@ -510,6 +510,7 @@ static void f_ch_open(typval_T *argvars, typval_T *rettv);
 static void f_ch_readraw(typval_T *argvars, typval_T *rettv);
 static void f_ch_sendexpr(typval_T *argvars, typval_T *rettv);
 static void f_ch_sendraw(typval_T *argvars, typval_T *rettv);
+static void f_ch_setoptions(typval_T *argvars, typval_T *rettv);
 static void f_ch_status(typval_T *argvars, typval_T *rettv);
 #endif
 static void f_changenr(typval_T *argvars, typval_T *rettv);
@@ -8131,6 +8132,7 @@ static struct fst
     {"ch_readraw",	1, 2, f_ch_readraw},
     {"ch_sendexpr",	2, 3, f_ch_sendexpr},
     {"ch_sendraw",	2, 3, f_ch_sendraw},
+    {"ch_setoptions",	2, 2, f_ch_setoptions},
     {"ch_status",	1, 1, f_ch_status},
 #endif
     {"changenr",	0, 0, f_changenr},
@@ -9870,44 +9872,91 @@ get_callback(typval_T *arg)
 }
 
 /*
- * Get the option entries from "dict", and parse them.
+ * Get the option entries from the dict in "tv", parse them and put the result
+ * in "opt".
+ * Only accept options in "supported".
  * If an option value is invalid return FAIL.
  */
     static int
-get_job_options(dict_T *dict, jobopt_T *opt)
+get_job_options(typval_T *tv, jobopt_T *opt, int supported)
 {
-    dictitem_T	*item;
+    typval_T	*item;
     char_u	*mode;
+    dict_T	*dict;
+    int		todo;
+    hashitem_T	*hi;
 
+    if (tv->v_type == VAR_UNKNOWN)
+	return OK;
+    if (tv->v_type != VAR_DICT)
+    {
+	EMSG(_(e_invarg));
+	return FAIL;
+    }
+    dict = tv->vval.v_dict;
     if (dict == NULL)
 	return OK;
 
-    if ((item = dict_find(dict, (char_u *)"mode", -1)) != NULL)
+    todo = (int)dict->dv_hashtab.ht_used;
-    {
+    for (hi = dict->dv_hashtab.ht_array; todo > 0; ++hi)
-	mode = get_tv_string(&item->di_tv);
+	if (!HASHITEM_EMPTY(hi))
-	if (STRCMP(mode, "nl") == 0)
-	    opt->jo_mode = MODE_NL;
-	else if (STRCMP(mode, "raw") == 0)
-	    opt->jo_mode = MODE_RAW;
-	else if (STRCMP(mode, "js") == 0)
-	    opt->jo_mode = MODE_JS;
-	else if (STRCMP(mode, "json") == 0)
-	    opt->jo_mode = MODE_JSON;
-	else
 	{
-	    EMSG2(_(e_invarg2), mode);
+	    item = &HI2DI(hi)->di_tv;
-	    return FAIL;
-	}
-    }
 
-    if ((item = dict_find(dict, (char_u *)"callback", -1)) != NULL)
+	    if (STRCMP(hi->hi_key, "mode") == 0)
-    {
+	    {
-	opt->jo_callback = get_callback(&item->di_tv);
+		if (!(supported & JO_MODE))
-	if (opt->jo_callback == NULL)
+		    break;
-	{
+		opt->jo_set |= JO_MODE;
-	    EMSG2(_(e_invarg2), "callback");
+		mode = get_tv_string(item);
-	    return FAIL;
+		if (STRCMP(mode, "nl") == 0)
+		    opt->jo_mode = MODE_NL;
+		else if (STRCMP(mode, "raw") == 0)
+		    opt->jo_mode = MODE_RAW;
+		else if (STRCMP(mode, "js") == 0)
+		    opt->jo_mode = MODE_JS;
+		else if (STRCMP(mode, "json") == 0)
+		    opt->jo_mode = MODE_JSON;
+		else
+		{
+		    EMSG2(_(e_invarg2), mode);
+		    return FAIL;
+		}
+	    }
+	    else if (STRCMP(hi->hi_key, "callback") == 0)
+	    {
+		if (!(supported & JO_CALLBACK))
+		    break;
+		opt->jo_set |= JO_CALLBACK;
+		opt->jo_callback = get_callback(item);
+		if (opt->jo_callback == NULL)
+		{
+		    EMSG2(_(e_invarg2), "callback");
+		    return FAIL;
+		}
+	    }
+	    else if (STRCMP(hi->hi_key, "waittime") == 0)
+	    {
+		if (!(supported & JO_WAITTIME))
+		    break;
+		opt->jo_set |= JO_WAITTIME;
+		opt->jo_waittime = get_tv_number(item);
+	    }
+	    else if (STRCMP(hi->hi_key, "timeout") == 0)
+	    {
+		if (!(supported & JO_TIMEOUT))
+		    break;
+		opt->jo_set |= JO_TIMEOUT;
+		opt->jo_timeout = get_tv_number(item);
+	    }
+	    else
+		break;
+	    --todo;
 	}
+    if (todo > 0)
+    {
+	EMSG2(_(e_invarg2), hi->hi_key);
+	return FAIL;
     }
 
     return OK;
@@ -10002,9 +10051,7 @@ f_ch_open(typval_T *argvars, typval_T *rettv)
     char_u	*p;
     char	*rest;
     int		port;
-    int		waittime = 0;
+    jobopt_T    opt;
-    int		timeout = 2000;
-    jobopt_T    options;
     channel_T	*channel;
 
     /* default: fail */
@@ -10035,33 +10082,26 @@ f_ch_open(typval_T *argvars, typval_T *rettv)
 	return;
     }
 
-    options.jo_mode = MODE_JSON;
+    /* parse options */
-    options.jo_callback = NULL;
+    opt.jo_mode = MODE_JSON;
-    if (argvars[1].v_type == VAR_DICT)
+    opt.jo_callback = NULL;
-    {
+    opt.jo_waittime = 0;
-	dict_T	    *dict = argvars[1].vval.v_dict;
+    opt.jo_timeout = 2000;
-	dictitem_T  *item;
+    if (get_job_options(&argvars[1], &opt,
-
+		JO_MODE + JO_CALLBACK + JO_WAITTIME + JO_TIMEOUT) == FAIL)
-	/* parse argdict */
+	return;
-	if (get_job_options(dict, &options) == FAIL)
+    if (opt.jo_timeout < 0)
-	    return;
-	if ((item = dict_find(dict, (char_u *)"waittime", -1)) != NULL)
-	    waittime = get_tv_number(&item->di_tv);
-	if ((item = dict_find(dict, (char_u *)"timeout", -1)) != NULL)
-	    timeout = get_tv_number(&item->di_tv);
-    }
-    if (timeout < 0)
     {
 	EMSG(_(e_invarg));
 	return;
     }
 
-    channel = channel_open((char *)address, port, waittime, NULL);
+    channel = channel_open((char *)address, port, opt.jo_waittime, NULL);
     if (channel != NULL)
     {
 	rettv->vval.v_channel = channel;
-	channel_set_options(channel, &options);
+	opt.jo_set = JO_ALL;
-	channel_set_timeout(channel, timeout);
+	channel_set_options(channel, &opt);
     }
 }
 
@@ -10077,30 +10117,13 @@ f_ch_readraw(typval_T *argvars, typval_T *rettv)
     rettv->v_type = VAR_STRING;
     rettv->vval.v_string = NULL;
 
+    /* TODO: use timeout from the options */
+
     channel = get_channel_arg(&argvars[0]);
     if (channel != NULL)
 	rettv->vval.v_string = channel_read_block(channel);
 }
 
-/*
- * "ch_status()" function
- */
-    static void
-f_ch_status(typval_T *argvars, typval_T *rettv)
-{
-    /* return an empty string by default */
-    rettv->v_type = VAR_STRING;
-
-    if (argvars[0].v_type != VAR_CHANNEL)
-    {
-	EMSG2(_(e_invarg2), get_tv_string(&argvars[0]));
-	rettv->vval.v_string = NULL;
-    }
-    else
-	rettv->vval.v_string = vim_strsave(
-			 (char_u *)channel_status(argvars[0].vval.v_channel));
-}
-
 /*
  * common for "sendexpr()" and "sendraw()"
  * Returns the channel if the caller should read the response.
@@ -10110,31 +10133,22 @@ f_ch_status(typval_T *argvars, typval_T *rettv)
 send_common(typval_T *argvars, char_u *text, int id, char *fun)
 {
     channel_T	*channel;
-    char_u	*callback = NULL;
+    jobopt_T	opt;
-    jobopt_T	options;
 
     channel = get_channel_arg(&argvars[0]);
     if (channel == NULL)
 	return NULL;
 
-    if (argvars[2].v_type != VAR_UNKNOWN)
+    opt.jo_callback = NULL;
-    {
+    if (get_job_options(&argvars[2], &opt, JO_CALLBACK) == FAIL)
-	if (argvars[2].v_type != VAR_DICT)
+	return NULL;
-	{
+
-	    EMSG(_(e_invarg));
-	    return NULL;
-	}
-	options.jo_callback = NULL;
-	if (get_job_options(argvars[2].vval.v_dict, &options) == FAIL)
-	    return NULL;
-	callback = options.jo_callback;
-    }
     /* Set the callback. An empty callback means no callback and not reading
      * the response. */
-    if (callback != NULL && *callback != NUL)
+    if (opt.jo_callback != NULL && *opt.jo_callback != NUL)
-	channel_set_req_callback(channel, callback, id);
+	channel_set_req_callback(channel, opt.jo_callback, id);
 
-    if (channel_send(channel, text, fun) == OK && callback == NULL)
+    if (channel_send(channel, text, fun) == OK && opt.jo_callback == NULL)
 	return channel;
     return NULL;
 }
@@ -10208,6 +10222,42 @@ f_ch_sendraw(typval_T *argvars, typval_T *rettv)
     if (channel != NULL)
 	rettv->vval.v_string = channel_read_block(channel);
 }
+
+/*
+ * "ch_setoptions()" function
+ */
+    static void
+f_ch_setoptions(typval_T *argvars, typval_T *rettv UNUSED)
+{
+    channel_T	*channel;
+    jobopt_T	opt;
+
+    channel = get_channel_arg(&argvars[0]);
+    if (channel == NULL)
+	return;
+    if (get_job_options(&argvars[1], &opt, JO_CALLBACK + JO_TIMEOUT) == FAIL)
+	return NULL;
+    channel_set_options(channel, &opt);
+}
+
+/*
+ * "ch_status()" function
+ */
+    static void
+f_ch_status(typval_T *argvars, typval_T *rettv)
+{
+    /* return an empty string by default */
+    rettv->v_type = VAR_STRING;
+
+    if (argvars[0].v_type != VAR_CHANNEL)
+    {
+	EMSG2(_(e_invarg2), get_tv_string(&argvars[0]));
+	rettv->vval.v_string = NULL;
+    }
+    else
+	rettv->vval.v_string = vim_strsave(
+			 (char_u *)channel_status(argvars[0].vval.v_channel));
+}
 #endif
 
 /*
@@ -14535,7 +14585,7 @@ f_job_start(typval_T *argvars UNUSED, typval_T *rettv)
 #else
     garray_T	ga;
 #endif
-    jobopt_T	options;
+    jobopt_T	opt;
 
     rettv->v_type = VAR_JOB;
     job = job_alloc();
@@ -14546,18 +14596,10 @@ f_job_start(typval_T *argvars UNUSED, typval_T *rettv)
     rettv->vval.v_job->jv_status = JOB_FAILED;
 
     /* Default mode is NL. */
-    options.jo_mode = MODE_NL;
+    opt.jo_mode = MODE_NL;
-    options.jo_callback = NULL;
+    opt.jo_callback = NULL;
-    if (argvars[1].v_type != VAR_UNKNOWN)
+    if (get_job_options(&argvars[1], &opt, JO_MODE + JO_CALLBACK) == FAIL)
-    {
+	return;
-	if (argvars[1].v_type != VAR_DICT)
-	{
-	    EMSG(_(e_invarg));
-	    return;
-	}
-	if (get_job_options(argvars[1].vval.v_dict, &options) == FAIL)
-	    return;
-    }
 
 #ifndef USE_ARGV
     ga_init2(&ga, (int)sizeof(char*), 20);
@@ -14639,12 +14681,12 @@ f_job_start(typval_T *argvars UNUSED, typval_T *rettv)
 	ga_clear(&ga);
     }
 # endif
-    mch_start_job(argv, job, &options);
+    mch_start_job(argv, job, &opt);
 #else
 # ifdef FEAT_CHANNEL
     ch_logs(NULL, "Starting job: %s", (char *)cmd);
 # endif
-    mch_start_job((char *)cmd, job, &options);
+    mch_start_job((char *)cmd, job, &opt);
 #endif
 
 theend:

src/proto/channel.pro

@@ -10,8 +10,6 @@ void channel_gui_register_all(void);
 channel_T *channel_open(char *hostname, int port_in, int waittime, void (*close_cb)(void));
 void channel_set_pipes(channel_T *channel, sock_T in, sock_T out, sock_T err);
 void channel_set_job(channel_T *channel, job_T *job);
-void channel_set_mode(channel_T *channel, ch_mode_T mode);
-void channel_set_timeout(channel_T *channel, int timeout);
 void channel_set_callback(channel_T *channel, char_u *callback);
 void channel_set_options(channel_T *channel, jobopt_T *options);
 void channel_set_req_callback(channel_T *channel, char_u *callback, int id);

src/structs.h

@@ -1372,13 +1372,23 @@ struct channel_S {
     int		ch_refcount;	/* reference count */
 };
 
+#define JO_MODE		1
+#define JO_CALLBACK	2
+#define JO_WAITTIME	4
+#define JO_TIMEOUT	8
+#define JO_ALL		0xffffff
+
 /*
  * Options for job and channel commands.
  */
 typedef struct
 {
-    ch_mode_T	jo_mode;	/* "mode" */
+    int		jo_set;		/* JO_ bits for values that were set */
-    char_u	*jo_callback;	/* "callback", not allocated! */
+
+    ch_mode_T	jo_mode;
+    char_u	*jo_callback;	/* not allocated! */
+    int		jo_waittime;
+    int		jo_timeout;
 } jobopt_T;
 
 

src/testdir/test_channel.vim

@@ -144,6 +144,12 @@ func s:communicate(port)
   endif
   call assert_equal('got it', s:responseMsg)
 
+  " check setting options (without testing the effect)
+  call ch_setoptions(handle, {'callback': 's:NotUsed'})
+  call ch_setoptions(handle, {'timeout': 111})
+  call assert_fails("call ch_setoptions(handle, {'waittime': 111})", "E475")
+  call assert_fails("call ch_setoptions(handle, {'mode': 'json'})", "E475")
+
   " Send an eval request that works.
   call assert_equal('ok', ch_sendexpr(handle, 'eval-works'))
   sleep 10m

src/version.c

@@ -747,6 +747,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1356,
 /**/
     1355,
 /**/