feat(jobs): jobstart(…,{term=true}), deprecate termopen() #31343

Problem:
`termopen` has long been a superficial wrapper around `jobstart`, and
has no real purpose. Also, `vim.system` and `nvim_open_term` presumably
will replace all features of `jobstart` and `termopen`, so centralizing
the logic will help with that.

Solution:
- Introduce `eval/deprecated.c`, where all deprecated eval funcs will live.
- Introduce "term" flag of `jobstart`.
- Deprecate `termopen`.

1 files changed, 90 insertions, 144 deletions

diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c
index 768d7664f7..23bfcff406 100644
--- a/src/nvim/eval/funcs.c
+++ b/src/nvim/eval/funcs.c
@@ -38,6 +38,7 @@
 #include "nvim/eval.h"
 #include "nvim/eval/buffer.h"
 #include "nvim/eval/decode.h"
+#include "nvim/eval/deprecated.h"
 #include "nvim/eval/encode.h"
 #include "nvim/eval/executor.h"
 #include "nvim/eval/funcs.h"
@@ -3840,8 +3841,8 @@ static const char *required_env_vars[] = {
   NULL
 };
 
-static dict_T *create_environment(const dictitem_T *job_env, const bool clear_env, const bool pty,
-                                  const char * const pty_term_name)
+dict_T *create_environment(const dictitem_T *job_env, const bool clear_env, const bool pty,
+                           const char * const pty_term_name)
 {
   dict_T *env = tv_dict_alloc();
 
@@ -3933,7 +3934,7 @@ static dict_T *create_environment(const dictitem_T *job_env, const bool clear_en
 }
 
 /// "jobstart()" function
-static void f_jobstart(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
+void f_jobstart(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
 {
   rettv->v_type = VAR_NUMBER;
   rettv->vval.v_number = 0;
@@ -3942,9 +3943,9 @@ static void f_jobstart(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
     return;
   }
 
+  const char *cmd;
   bool executable = true;
-  char **argv = tv_to_argv(&argvars[0], NULL, &executable);
-  dict_T *env = NULL;
+  char **argv = tv_to_argv(&argvars[0], &cmd, &executable);
   if (!argv) {
     rettv->vval.v_number = executable ? 0 : -1;
     return;  // Did error message in tv_to_argv.
@@ -3961,6 +3962,7 @@ static void f_jobstart(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
   bool detach = false;
   bool rpc = false;
   bool pty = false;
+  bool term = false;
   bool clear_env = false;
   bool overlapped = false;
   ChannelStdinMode stdin_mode = kChannelStdinPipe;
@@ -3974,7 +3976,8 @@ static void f_jobstart(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
 
     detach = tv_dict_get_number(job_opts, "detach") != 0;
     rpc = tv_dict_get_number(job_opts, "rpc") != 0;
-    pty = tv_dict_get_number(job_opts, "pty") != 0;
+    term = tv_dict_get_number(job_opts, "term") != 0;
+    pty = term || tv_dict_get_number(job_opts, "pty") != 0;
     clear_env = tv_dict_get_number(job_opts, "clear_env") != 0;
     overlapped = tv_dict_get_number(job_opts, "overlapped") != 0;
 
@@ -3989,6 +3992,14 @@ static void f_jobstart(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
       }
     }
 
+    dictitem_T *const job_term = tv_dict_find(job_opts, S_LEN("term"));
+    if (job_term && VAR_BOOL != job_term->di_tv.v_type) {
+      // Restrict "term" field to boolean, in case we want to allow buffer numbers in the future.
+      semsg(_(e_invarg2), "'term' must be Boolean");
+      shell_free_argv(argv);
+      return;
+    }
+
     if (pty && rpc) {
       semsg(_(e_invarg2), "job cannot have both 'pty' and 'rpc' options set");
       shell_free_argv(argv);
@@ -4032,24 +4043,87 @@ static void f_jobstart(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
   uint16_t height = 0;
   char *term_name = NULL;
 
-  if (pty) {
-    width = (uint16_t)tv_dict_get_number(job_opts, "width");
-    height = (uint16_t)tv_dict_get_number(job_opts, "height");
-    // Legacy method, before env option existed, to specify $TERM.  No longer
-    // documented, but still usable to avoid breaking scripts.
-    term_name = tv_dict_get_string(job_opts, "TERM", false);
-    if (!term_name) {
-      term_name = "ansi";
+  if (term) {
+    if (text_locked()) {
+      text_locked_msg();
+      shell_free_argv(argv);
+      return;
     }
+    if (curbuf->b_changed) {
+      emsg(_("jobstart(...,{term=true}) requires unmodified buffer"));
+      shell_free_argv(argv);
+      return;
+    }
+    assert(!rpc);
+    term_name = "xterm-256color";
+    cwd = cwd ? cwd : ".";
+    overlapped = false;
+    detach = false;
+    stdin_mode = kChannelStdinPipe;
+    width = (uint16_t)MAX(0, curwin->w_width_inner - win_col_off(curwin));
+    height = (uint16_t)curwin->w_height_inner;
   }
 
-  env = create_environment(job_env, clear_env, pty, term_name);
+  if (pty) {
+    width = width ? width : (uint16_t)tv_dict_get_number(job_opts, "width");
+    height = height ? height : (uint16_t)tv_dict_get_number(job_opts, "height");
+    // Deprecated TERM field is from before `env` option existed.
+    term_name = term_name ? term_name : tv_dict_get_string(job_opts, "TERM", false);
+    term_name = term_name ? term_name : "ansi";
+  }
 
+  dict_T *env = create_environment(job_env, clear_env, pty, term_name);
   Channel *chan = channel_job_start(argv, NULL, on_stdout, on_stderr, on_exit, pty,
                                     rpc, overlapped, detach, stdin_mode, cwd,
                                     width, height, env, &rettv->vval.v_number);
-  if (chan) {
+  if (!chan) {
+    return;
+  } else if (!term) {
     channel_create_event(chan, NULL);
+  } else {
+    if (rettv->vval.v_number <= 0) {
+      return;
+    }
+
+    int pid = chan->stream.pty.proc.pid;
+
+    // "./…" => "/home/foo/…"
+    vim_FullName(cwd, NameBuff, sizeof(NameBuff), false);
+    // "/home/foo/…" => "~/…"
+    size_t len = home_replace(NULL, NameBuff, IObuff, sizeof(IObuff), true);
+    // Trim slash.
+    if (len != 1 && (IObuff[len - 1] == '\\' || IObuff[len - 1] == '/')) {
+      IObuff[len - 1] = NUL;
+    }
+
+    if (len == 1 && IObuff[0] == '/') {
+      // Avoid ambiguity in the URI when CWD is root directory.
+      IObuff[1] = '.';
+      IObuff[2] = NUL;
+    }
+
+    // Terminal URI: "term://$CWD//$PID:$CMD"
+    snprintf(NameBuff, sizeof(NameBuff), "term://%s//%d:%s", IObuff, pid, cmd);
+    // Buffer has no terminal associated yet; unset 'swapfile' to ensure no swapfile is created.
+    curbuf->b_p_swf = false;
+
+    apply_autocmds(EVENT_BUFFILEPRE, NULL, NULL, false, curbuf);
+    setfname(curbuf, NameBuff, NULL, true);
+    apply_autocmds(EVENT_BUFFILEPOST, NULL, NULL, false, curbuf);
+
+    Error err = ERROR_INIT;
+    // Set (deprecated) buffer-local vars (prefer 'channel' buffer-local option).
+    dict_set_var(curbuf->b_vars, cstr_as_string("terminal_job_id"),
+                 INTEGER_OBJ((Integer)chan->id), false, false, NULL, &err);
+    api_clear_error(&err);
+    dict_set_var(curbuf->b_vars, cstr_as_string("terminal_job_pid"),
+                 INTEGER_OBJ(pid), false, false, NULL, &err);
+    api_clear_error(&err);
+
+    channel_incref(chan);
+    channel_terminal_open(curbuf, chan);
+    channel_create_event(chan, NULL);
+    channel_decref(chan);
   }
 }
 
@@ -8133,134 +8207,6 @@ static void f_taglist(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
            (char *)tag_pattern, (char *)fname);
 }
 
-/// "termopen(cmd[, cwd])" function
-static void f_termopen(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
-{
-  if (check_secure()) {
-    return;
-  }
-  if (text_locked()) {
-    text_locked_msg();
-    return;
-  }
-  if (curbuf->b_changed) {
-    emsg(_("Can only call this function in an unmodified buffer"));
-    return;
-  }
-
-  const char *cmd;
-  bool executable = true;
-  char **argv = tv_to_argv(&argvars[0], &cmd, &executable);
-  if (!argv) {
-    rettv->vval.v_number = executable ? 0 : -1;
-    return;  // Did error message in tv_to_argv.
-  }
-
-  if (argvars[1].v_type != VAR_DICT && argvars[1].v_type != VAR_UNKNOWN) {
-    // Wrong argument type
-    semsg(_(e_invarg2), "expected dictionary");
-    shell_free_argv(argv);
-    return;
-  }
-
-  CallbackReader on_stdout = CALLBACK_READER_INIT;
-  CallbackReader on_stderr = CALLBACK_READER_INIT;
-  Callback on_exit = CALLBACK_NONE;
-  dict_T *job_opts = NULL;
-  const char *cwd = ".";
-  dict_T *env = NULL;
-  const bool pty = true;
-  bool clear_env = false;
-  dictitem_T *job_env = NULL;
-
-  if (argvars[1].v_type == VAR_DICT) {
-    job_opts = argvars[1].vval.v_dict;
-
-    const char *const new_cwd = tv_dict_get_string(job_opts, "cwd", false);
-    if (new_cwd && *new_cwd != NUL) {
-      cwd = new_cwd;
-      // The new cwd must be a directory.
-      if (!os_isdir(cwd)) {
-        semsg(_(e_invarg2), "expected valid directory");
-        shell_free_argv(argv);
-        return;
-      }
-    }
-
-    job_env = tv_dict_find(job_opts, S_LEN("env"));
-    if (job_env && job_env->di_tv.v_type != VAR_DICT) {
-      semsg(_(e_invarg2), "env");
-      shell_free_argv(argv);
-      return;
-    }
-
-    clear_env = tv_dict_get_number(job_opts, "clear_env") != 0;
-
-    if (!common_job_callbacks(job_opts, &on_stdout, &on_stderr, &on_exit)) {
-      shell_free_argv(argv);
-      return;
-    }
-  }
-
-  env = create_environment(job_env, clear_env, pty, "xterm-256color");
-
-  const bool rpc = false;
-  const bool overlapped = false;
-  const bool detach = false;
-  ChannelStdinMode stdin_mode = kChannelStdinPipe;
-  uint16_t term_width = (uint16_t)MAX(0, curwin->w_width_inner - win_col_off(curwin));
-  Channel *chan = channel_job_start(argv, NULL, on_stdout, on_stderr, on_exit,
-                                    pty, rpc, overlapped, detach, stdin_mode,
-                                    cwd, term_width, (uint16_t)curwin->w_height_inner,
-                                    env, &rettv->vval.v_number);
-  if (rettv->vval.v_number <= 0) {
-    return;
-  }
-
-  int pid = chan->stream.pty.proc.pid;
-
-  // "./…" => "/home/foo/…"
-  vim_FullName(cwd, NameBuff, sizeof(NameBuff), false);
-  // "/home/foo/…" => "~/…"
-  size_t len = home_replace(NULL, NameBuff, IObuff, sizeof(IObuff), true);
-  // Trim slash.
-  if (len != 1 && (IObuff[len - 1] == '\\' || IObuff[len - 1] == '/')) {
-    IObuff[len - 1] = NUL;
-  }
-
-  if (len == 1 && IObuff[0] == '/') {
-    // Avoid ambiguity in the URI when CWD is root directory.
-    IObuff[1] = '.';
-    IObuff[2] = NUL;
-  }
-
-  // Terminal URI: "term://$CWD//$PID:$CMD"
-  snprintf(NameBuff, sizeof(NameBuff), "term://%s//%d:%s",
-           IObuff, pid, cmd);
-  // at this point the buffer has no terminal instance associated yet, so unset
-  // the 'swapfile' option to ensure no swap file will be created
-  curbuf->b_p_swf = false;
-
-  apply_autocmds(EVENT_BUFFILEPRE, NULL, NULL, false, curbuf);
-  setfname(curbuf, NameBuff, NULL, true);
-  apply_autocmds(EVENT_BUFFILEPOST, NULL, NULL, false, curbuf);
-
-  // Save the job id and pid in b:terminal_job_{id,pid}
-  Error err = ERROR_INIT;
-  // deprecated: use 'channel' buffer option
-  dict_set_var(curbuf->b_vars, cstr_as_string("terminal_job_id"),
-               INTEGER_OBJ((Integer)chan->id), false, false, NULL, &err);
-  api_clear_error(&err);
-  dict_set_var(curbuf->b_vars, cstr_as_string("terminal_job_pid"),
-               INTEGER_OBJ(pid), false, false, NULL, &err);
-  api_clear_error(&err);
-
-  channel_incref(chan);
-  channel_terminal_open(curbuf, chan);
-  channel_create_event(chan, NULL);
-  channel_decref(chan);
-}
-
 /// "timer_info([timer])" function
 static void f_timer_info(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
 {