From 34b6a3d944512fd544a884b018297a44aff928ab Mon Sep 17 00:00:00 2001
From: "Justin M. Keyes" <justinkz@gmail.com>
Date: Sun, 6 May 2018 21:14:25 +0200
Subject: doc

---
 src/nvim/api/private/helpers.c |  4 +---
 src/nvim/api/vim.c             | 19 +++++++++----------
 src/nvim/ex_eval.c             | 31 +++++++++++++++----------------
 3 files changed, 25 insertions(+), 29 deletions(-)

(limited to 'src')

diff --git a/src/nvim/api/private/helpers.c b/src/nvim/api/private/helpers.c
index 12a4279dd7..637e24575d 100644
--- a/src/nvim/api/private/helpers.c
+++ b/src/nvim/api/private/helpers.c
@@ -120,9 +120,7 @@ bool try_end(Error *err)
   // try_enter/try_leave.
   trylevel--;
 
-  // Without this it stops processing all subsequent VimL commands and
-  // generates strange error messages if I e.g. try calling Test() in a
-  // cycle
+  // Set by emsg(), affects aborting().  See also enter_cleanup().
   did_emsg = false;
 
   if (got_int) {
diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c
index 3679d1e569..5b9ae141b9 100644
--- a/src/nvim/api/vim.c
+++ b/src/nvim/api/vim.c
@@ -934,27 +934,26 @@ Array nvim_get_api_info(uint64_t channel_id)
   return rv;
 }
 
-/// Call many api methods atomically
+/// Calls many API methods atomically.
 ///
-/// This has two main usages: Firstly, to perform several requests from an
-/// async context atomically, i.e. without processing requests from other rpc
-/// clients or redrawing or allowing user interaction in between. Note that api
-/// methods that could fire autocommands or do event processing still might do
-/// so. For instance invoking the :sleep command might call timer callbacks.
-/// Secondly, it can be used to reduce rpc overhead (roundtrips) when doing
-/// many requests in sequence.
+/// This has two main usages:
+/// 1. To perform several requests from an async context atomically, i.e.
+///    without interleaving redraws, RPC requests from other clients, or user
+///    interactions (however API methods may trigger autocommands or event
+///    processing which have such side-effects, e.g. |:sleep| may wake timers).
+/// 2. To minimize RPC overhead (roundtrips) of a sequence of many requests.
 ///
 /// @param calls an array of calls, where each call is described by an array
 /// with two elements: the request name, and an array of arguments.
 /// @param[out] err Details of a validation error of the nvim_multi_request call
-/// itself, i e malformatted `calls` parameter. Errors from called methods will
+/// itself, i.e. malformed `calls` parameter. Errors from called methods will
 /// be indicated in the return value, see below.
 ///
 /// @return an array with two elements. The first is an array of return
 /// values. The second is NIL if all calls succeeded. If a call resulted in
 /// an error, it is a three-element array with the zero-based index of the call
 /// which resulted in an error, the error type and the error message. If an
-/// error ocurred, the values from all preceding calls will still be returned.
+/// error occurred, the values from all preceding calls will still be returned.
 Array nvim_call_atomic(uint64_t channel_id, Array calls, Error *err)
   FUNC_API_SINCE(1) FUNC_API_REMOTE_ONLY
 {
diff --git a/src/nvim/ex_eval.c b/src/nvim/ex_eval.c
index e23945c842..7f7851f078 100644
--- a/src/nvim/ex_eval.c
+++ b/src/nvim/ex_eval.c
@@ -28,22 +28,21 @@
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "ex_eval.c.generated.h"
 #endif
-/*
- * Exception handling terms:
- *
- *	:try		":try" command		\
- *	    ...		try block		|
- *	:catch RE	":catch" command	|
- *	    ...		catch clause		|- try conditional
- *	:finally	":finally" command	|
- *	    ...		finally clause		|
- *	:endtry		":endtry" command	/
- *
- * The try conditional may have any number of catch clauses and at most one
- * finally clause.  A ":throw" command can be inside the try block, a catch
- * clause, the finally clause, or in a function called or script sourced from
- * there or even outside the try conditional.  Try conditionals may be nested.
- */
+
+// Exception handling terms:
+//
+//      :try            ":try" command         ─┐
+//          ...         try block               │
+//      :catch RE       ":catch" command        │
+//          ...         catch clause            ├─ try conditional
+//      :finally        ":finally" command      │
+//          ...         finally clause          │
+//      :endtry         ":endtry" command      ─┘
+//
+// The try conditional may have any number of catch clauses and at most one
+// finally clause.  A ":throw" command can be inside the try block, a catch
+// clause, the finally clause, or in a function called or script sourced from
+// there or even outside the try conditional.  Try conditionals may be nested.
 
 // Configuration whether an exception is thrown on error or interrupt.  When
 // the preprocessor macros below evaluate to FALSE, an error (did_emsg) or
-- 
cgit 


From c9f3174075a7168f10fabf806891ef59ee3b13d4 Mon Sep 17 00:00:00 2001
From: "Justin M. Keyes" <justinkz@gmail.com>
Date: Mon, 7 May 2018 03:24:01 +0200
Subject: API: return non-generic VimL errors

- Return VimL errors instead of generic errors for:
  - nvim_call_function
  - nvim_call_dict_function
- Fix tests which were silently broken before this change.

This violates #6150 where we agreed not to translate API errors.  But
that can be fixed later.
---
 src/nvim/api/vim.c | 30 +++++++++++++++++++-----------
 src/nvim/eval.c    | 29 +++++++++++++++++------------
 src/nvim/message.c |  4 ++++
 src/nvim/message.h |  4 ++++
 4 files changed, 44 insertions(+), 23 deletions(-)

(limited to 'src')

diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c
index 5b9ae141b9..2d1e2bbe94 100644
--- a/src/nvim/api/vim.c
+++ b/src/nvim/api/vim.c
@@ -46,8 +46,7 @@
 
 /// Executes an ex-command.
 ///
-/// On parse error: forwards the Vim error; does not update v:errmsg.
-/// On runtime error: forwards the Vim error; does not update v:errmsg.
+/// On execution error: fails with VimL error, does not update v:errmsg.
 ///
 /// @param command  Ex-command string
 /// @param[out] err Error details (Vim error), if any
@@ -103,7 +102,8 @@ Dictionary nvim_get_hl_by_id(Integer hl_id, Boolean rgb, Error *err)
 }
 
 /// Passes input keys to Nvim.
-/// On VimL error: Does not fail, but updates v:errmsg.
+///
+/// On execution error: does not fail, but updates v:errmsg.
 ///
 /// @param keys         to be typed
 /// @param mode         mapping options
@@ -169,7 +169,8 @@ void nvim_feedkeys(String keys, String mode, Boolean escape_csi)
 }
 
 /// Passes keys to Nvim as raw user-input.
-/// On VimL error: Does not fail, but updates v:errmsg.
+///
+/// On execution error: does not fail, but updates v:errmsg.
 ///
 /// Unlike `nvim_feedkeys`, this uses a lower-level input buffer and the call
 /// is not deferred. This is the most reliable way to send real user input.
@@ -213,8 +214,7 @@ String nvim_replace_termcodes(String str, Boolean from_part, Boolean do_lt,
 /// Executes an ex-command and returns its (non-error) output.
 /// Shell |:!| output is not captured.
 ///
-/// On parse error: forwards the Vim error; does not update v:errmsg.
-/// On runtime error: forwards the Vim error; does not update v:errmsg.
+/// On execution error: fails with VimL error, does not update v:errmsg.
 ///
 /// @param command  Ex-command string
 /// @param[out] err Error details (Vim error), if any
@@ -259,7 +259,8 @@ theend:
 
 /// Evaluates a VimL expression (:help expression).
 /// Dictionaries and Lists are recursively expanded.
-/// On VimL error: Returns a generic error; v:errmsg is not updated.
+///
+/// On execution error: fails with generic error; v:errmsg is not updated.
 ///
 /// @param expr     VimL expression string
 /// @param[out] err Error details, if any
@@ -331,18 +332,23 @@ static Object _call_function(String fn, Array args, dict_T *self, Error *err)
   }
 
   try_start();
-  // Call the function
+  msg_first_ignored_err = NULL;
+
   typval_T rettv;
   int dummy;
   int r = call_func((char_u *)fn.data, (int)fn.size, &rettv, (int)args.size,
                     vim_args, NULL, curwin->w_cursor.lnum,
                     curwin->w_cursor.lnum, &dummy, true, NULL, self);
-  if (r == FAIL) {
-    api_set_error(err, kErrorTypeException, "Error calling function");
+  // call_func() retval is deceptive; must also check did_emsg et al.
+  if (msg_first_ignored_err
+      && (r == FAIL || (did_emsg && force_abort && !current_exception))) {
+    api_set_error(err, kErrorTypeException, msg_first_ignored_err);
   }
   if (!try_end(err)) {
     rv = vim_to_object(&rettv);
   }
+  xfree(msg_first_ignored_err);
+  msg_first_ignored_err = NULL;
   tv_clear(&rettv);
 
 free_vim_args:
@@ -355,7 +361,7 @@ free_vim_args:
 
 /// Calls a VimL function with the given arguments.
 ///
-/// On VimL error: Returns a generic error; v:errmsg is not updated.
+/// On execution error: fails with VimL error, does not update v:errmsg.
 ///
 /// @param fn       Function to call
 /// @param args     Function arguments packed in an Array
@@ -369,6 +375,8 @@ Object nvim_call_function(String fn, Array args, Error *err)
 
 /// Calls a VimL |Dictionary-function| with the given arguments.
 ///
+/// On execution error: fails with VimL error, does not update v:errmsg.
+///
 /// @param dict Dictionary, or String evaluating to a VimL |self| dict
 /// @param fn Name of the function defined on the VimL dict
 /// @param args Function arguments packed in an Array
diff --git a/src/nvim/eval.c b/src/nvim/eval.c
index cc29496968..3c408a4381 100644
--- a/src/nvim/eval.c
+++ b/src/nvim/eval.c
@@ -6241,20 +6241,21 @@ bool set_ref_in_func(char_u *name, ufunc_T *fp_in, int copyID)
 /// invoked function uses them. It is called like this:
 ///   new_argcount = argv_func(current_argcount, argv, called_func_argcount)
 ///
-/// Return FAIL when the function can't be called,  OK otherwise.
-/// Also returns OK when an error was encountered while executing the function.
+/// @return FAIL if function cannot be called, else OK (even if an error
+///         occurred while executing the function! Use `msg_first_ignored_err`
+///         to get the error)
 int
 call_func(
     const char_u *funcname,         // name of the function
     int len,                        // length of "name"
-    typval_T *rettv,                // return value goes here
+    typval_T *rettv,                // [out] value goes here
     int argcount_in,                // number of "argvars"
     typval_T *argvars_in,           // vars for arguments, must have "argcount"
                                     // PLUS ONE elements!
     ArgvFunc argv_func,             // function to fill in argvars
     linenr_T firstline,             // first line of range
     linenr_T lastline,              // last line of range
-    int *doesrange,                 // return: function handled range
+    int *doesrange,                 // [out] function handled range
     bool evaluate,
     partial_T *partial,             // optional, can be NULL
     dict_T *selfdict_in             // Dictionary for "self"
@@ -6428,21 +6429,25 @@ call_func(
   return ret;
 }
 
-/*
- * Give an error message with a function name.  Handle <SNR> things.
- * "ermsg" is to be passed without translation, use N_() instead of _().
- */
+/// Give an error message with a function name.  Handle <SNR> things.
+///
+/// @param ermsg must be passed without translation (use N_() instead of _()).
+/// @param name function name
 static void emsg_funcname(char *ermsg, char_u *name)
 {
-  char_u      *p;
+  char_u *p;
 
-  if (*name == K_SPECIAL)
+  if (*name == K_SPECIAL) {
     p = concat_str((char_u *)"<SNR>", name + 3);
-  else
+  } else {
     p = name;
+  }
+
   EMSG2(_(ermsg), p);
-  if (p != name)
+
+  if (p != name) {
     xfree(p);
+  }
 }
 
 /*
diff --git a/src/nvim/message.c b/src/nvim/message.c
index 7ca82c2878..bcfff90d7d 100644
--- a/src/nvim/message.c
+++ b/src/nvim/message.c
@@ -67,6 +67,7 @@ static char_u   *confirm_msg_tail;              /* tail of confirm_msg */
 
 MessageHistoryEntry *first_msg_hist = NULL;
 MessageHistoryEntry *last_msg_hist = NULL;
+char *msg_first_ignored_err = NULL;
 static int msg_hist_len = 0;
 
 static FILE *verbose_fd = NULL;
@@ -504,6 +505,9 @@ int emsg(const char_u *s_)
     if (cause_errthrow((char_u *)s, severe, &ignore) == true) {
       if (!ignore) {
         did_emsg = true;
+        if (msg_first_ignored_err == NULL) {
+          msg_first_ignored_err = xstrdup(s);
+        }
       }
       return true;
     }
diff --git a/src/nvim/message.h b/src/nvim/message.h
index 82935a36a9..eede0ed9b1 100644
--- a/src/nvim/message.h
+++ b/src/nvim/message.h
@@ -85,6 +85,10 @@ extern MessageHistoryEntry *first_msg_hist;
 /// Last message
 extern MessageHistoryEntry *last_msg_hist;
 
+/// Abort-causing non-exception error ignored by emsg(), needed by callers
+/// (RPC API) of call_func() to get error details when messages are disabled.
+extern char *msg_first_ignored_err;
+
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "message.h.generated.h"
 #endif
-- 
cgit 


From 32b0470b03b3892a4ed6c4bfec0d4a5527d996b1 Mon Sep 17 00:00:00 2001
From: "Justin M. Keyes" <justinkz@gmail.com>
Date: Wed, 9 May 2018 03:02:12 +0200
Subject: API: better way to capture abort-causing non-exception errors

This condition is not perfectly reliable:
    (did_emsg && force_abort && !current_exception)

The more proper way to check for abort-causing non-exception errors is
to set up `msg_list` using the "pattern" given by do_cmdline().
---
 src/nvim/api/vim.c | 42 ++++++++++++++++++++++++++----------------
 src/nvim/eval.c    |  4 ++--
 src/nvim/message.c |  4 ----
 src/nvim/message.h |  4 ----
 4 files changed, 28 insertions(+), 26 deletions(-)

(limited to 'src')

diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c
index 2d1e2bbe94..7f8c10fcd1 100644
--- a/src/nvim/api/vim.c
+++ b/src/nvim/api/vim.c
@@ -260,7 +260,7 @@ theend:
 /// Evaluates a VimL expression (:help expression).
 /// Dictionaries and Lists are recursively expanded.
 ///
-/// On execution error: fails with generic error; v:errmsg is not updated.
+/// On execution error: fails with VimL error, does not update v:errmsg.
 ///
 /// @param expr     VimL expression string
 /// @param[out] err Error details, if any
@@ -269,7 +269,6 @@ Object nvim_eval(String expr, Error *err)
   FUNC_API_SINCE(1)
 {
   Object rv = OBJECT_INIT;
-  // Evaluate the expression
   try_start();
 
   typval_T rettv;
@@ -278,11 +277,9 @@ Object nvim_eval(String expr, Error *err)
   }
 
   if (!try_end(err)) {
-    // No errors, convert the result
     rv = vim_to_object(&rettv);
   }
 
-  // Free the Vim object
   tv_clear(&rettv);
 
   return rv;
@@ -315,7 +312,9 @@ Object nvim_execute_lua(String code, Array args, Error *err)
 /// @return Result of the function call
 static Object _call_function(String fn, Array args, dict_T *self, Error *err)
 {
+  static int recursive = 0;  // recursion depth
   Object rv = OBJECT_INIT;
+
   if (args.size > MAX_FUNC_ARGS) {
     api_set_error(err, kErrorTypeValidation,
                   "Function called with too many arguments");
@@ -331,25 +330,36 @@ static Object _call_function(String fn, Array args, dict_T *self, Error *err)
     }
   }
 
+  // `msg_list` controls the collection of abort-causing non-exception errors,
+  // which would otherwise be ignored.  This pattern is from do_cmdline().
+  struct msglist **saved_msg_list = msg_list;
+  struct msglist *private_msg_list;
+  msg_list = &private_msg_list;
+  private_msg_list = NULL;
+
+  // Initialize `force_abort`  and `suppress_errthrow` at the top level.
+  if (!recursive) {
+    force_abort = false;
+    suppress_errthrow = false;
+    current_exception = NULL;
+    // `did_emsg` is set by emsg(), which cancels execution.
+    did_emsg = false;
+  }
+  recursive++;
   try_start();
-  msg_first_ignored_err = NULL;
-
   typval_T rettv;
   int dummy;
-  int r = call_func((char_u *)fn.data, (int)fn.size, &rettv, (int)args.size,
-                    vim_args, NULL, curwin->w_cursor.lnum,
-                    curwin->w_cursor.lnum, &dummy, true, NULL, self);
-  // call_func() retval is deceptive; must also check did_emsg et al.
-  if (msg_first_ignored_err
-      && (r == FAIL || (did_emsg && force_abort && !current_exception))) {
-    api_set_error(err, kErrorTypeException, msg_first_ignored_err);
-  }
+  // call_func() retval is deceptive, ignore it.  Instead we set `msg_list`
+  // (see above) to capture abort-causing non-exception errors.
+  (void)call_func((char_u *)fn.data, (int)fn.size, &rettv, (int)args.size,
+                  vim_args, NULL, curwin->w_cursor.lnum, curwin->w_cursor.lnum,
+                  &dummy, true, NULL, self);
   if (!try_end(err)) {
     rv = vim_to_object(&rettv);
   }
-  xfree(msg_first_ignored_err);
-  msg_first_ignored_err = NULL;
   tv_clear(&rettv);
+  msg_list = saved_msg_list;  // Restore the exception context.
+  recursive--;
 
 free_vim_args:
   while (i > 0) {
diff --git a/src/nvim/eval.c b/src/nvim/eval.c
index 3c408a4381..45a1776c86 100644
--- a/src/nvim/eval.c
+++ b/src/nvim/eval.c
@@ -6242,8 +6242,8 @@ bool set_ref_in_func(char_u *name, ufunc_T *fp_in, int copyID)
 ///   new_argcount = argv_func(current_argcount, argv, called_func_argcount)
 ///
 /// @return FAIL if function cannot be called, else OK (even if an error
-///         occurred while executing the function! Use `msg_first_ignored_err`
-///         to get the error)
+///         occurred while executing the function! Set `msg_list` to capture
+///         the error, see do_cmdline()).
 int
 call_func(
     const char_u *funcname,         // name of the function
diff --git a/src/nvim/message.c b/src/nvim/message.c
index bcfff90d7d..7ca82c2878 100644
--- a/src/nvim/message.c
+++ b/src/nvim/message.c
@@ -67,7 +67,6 @@ static char_u   *confirm_msg_tail;              /* tail of confirm_msg */
 
 MessageHistoryEntry *first_msg_hist = NULL;
 MessageHistoryEntry *last_msg_hist = NULL;
-char *msg_first_ignored_err = NULL;
 static int msg_hist_len = 0;
 
 static FILE *verbose_fd = NULL;
@@ -505,9 +504,6 @@ int emsg(const char_u *s_)
     if (cause_errthrow((char_u *)s, severe, &ignore) == true) {
       if (!ignore) {
         did_emsg = true;
-        if (msg_first_ignored_err == NULL) {
-          msg_first_ignored_err = xstrdup(s);
-        }
       }
       return true;
     }
diff --git a/src/nvim/message.h b/src/nvim/message.h
index eede0ed9b1..82935a36a9 100644
--- a/src/nvim/message.h
+++ b/src/nvim/message.h
@@ -85,10 +85,6 @@ extern MessageHistoryEntry *first_msg_hist;
 /// Last message
 extern MessageHistoryEntry *last_msg_hist;
 
-/// Abort-causing non-exception error ignored by emsg(), needed by callers
-/// (RPC API) of call_func() to get error details when messages are disabled.
-extern char *msg_first_ignored_err;
-
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "message.h.generated.h"
 #endif
-- 
cgit 


From 2326a4ac3a9dfdedba6fd4e1c68491b071fdc57b Mon Sep 17 00:00:00 2001
From: "Justin M. Keyes" <justinkz@gmail.com>
Date: Wed, 9 May 2018 03:23:18 +0200
Subject: API: nvim_eval(): return non-generic VimL errors

Use the same pattern as nvim_call_function (_call_function).
---
 src/nvim/api/vim.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

(limited to 'src')

diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c
index 7f8c10fcd1..1f7f13c831 100644
--- a/src/nvim/api/vim.c
+++ b/src/nvim/api/vim.c
@@ -268,11 +268,30 @@ theend:
 Object nvim_eval(String expr, Error *err)
   FUNC_API_SINCE(1)
 {
+  static int recursive = 0;  // recursion depth
   Object rv = OBJECT_INIT;
+
+  // `msg_list` controls the collection of abort-causing non-exception errors,
+  // which would otherwise be ignored.  This pattern is from do_cmdline().
+  struct msglist **saved_msg_list = msg_list;
+  struct msglist *private_msg_list;
+  msg_list = &private_msg_list;
+  private_msg_list = NULL;
+
+  // Initialize `force_abort`  and `suppress_errthrow` at the top level.
+  if (!recursive) {
+    force_abort = false;
+    suppress_errthrow = false;
+    current_exception = NULL;
+    // `did_emsg` is set by emsg(), which cancels execution.
+    did_emsg = false;
+  }
+  recursive++;
   try_start();
 
   typval_T rettv;
   if (eval0((char_u *)expr.data, &rettv, NULL, true) == FAIL) {
+    // This generic error should be overwritten by try_end() since #8371.
     api_set_error(err, kErrorTypeException, "Failed to evaluate expression");
   }
 
@@ -281,6 +300,8 @@ Object nvim_eval(String expr, Error *err)
   }
 
   tv_clear(&rettv);
+  msg_list = saved_msg_list;  // Restore the exception context.
+  recursive--;
 
   return rv;
 }
-- 
cgit 


From cb8ea55d7174c44ece72098eb853aacd608a0aa3 Mon Sep 17 00:00:00 2001
From: "Justin M. Keyes" <justinkz@gmail.com>
Date: Wed, 9 May 2018 21:44:38 +0200
Subject: nvim_eval: fix memory leak

---
 src/nvim/api/vim.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

(limited to 'src')

diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c
index 1f7f13c831..fd9cf332d5 100644
--- a/src/nvim/api/vim.c
+++ b/src/nvim/api/vim.c
@@ -290,13 +290,15 @@ Object nvim_eval(String expr, Error *err)
   try_start();
 
   typval_T rettv;
-  if (eval0((char_u *)expr.data, &rettv, NULL, true) == FAIL) {
-    // This generic error should be overwritten by try_end() since #8371.
-    api_set_error(err, kErrorTypeException, "Failed to evaluate expression");
-  }
+  int ok = eval0((char_u *)expr.data, &rettv, NULL, true);
 
   if (!try_end(err)) {
-    rv = vim_to_object(&rettv);
+    if (ok == FAIL) {
+      // Should never happen, try_end() should get the error. #8371
+      api_set_error(err, kErrorTypeException, "Failed to evaluate expression");
+    } else {
+      rv = vim_to_object(&rettv);
+    }
   }
 
   tv_clear(&rettv);
-- 
cgit