properly document some functions in expressions.c and simplified code

* Practically all calls to teco_expressions_args() must be preceded by teco_expressions_eval().
* In code paths where we know that teco_expressions_args() > 0, it is safe
  to call teco_expressions_pop_num(0) instead of teco_expressions_pop_num_calc().
  This is both easier and faster.
* teco_expressions_pop_num_calc() is for simple applications where you just want to get
  a command argument with default (implied) values.
  Since it includes teco_expressions_eval(), we can avoid superfluous calls.
* -EC...$ turned out to be broken and is fixed now.
  A test case has been added.

10 files changed, 81 insertions, 93 deletions

diff --git a/src/core-commands.c b/src/core-commands.c
index abd3a22..1a4b22a 100644
--- a/src/core-commands.c
+++ b/src/core-commands.c
@@ -259,13 +259,9 @@ teco_state_start_backslash(teco_machine_main_t *ctx, GError **error)
 		return;
 
 	if (teco_expressions_args()) {
-		teco_int_t value;
-
-		if (!teco_expressions_pop_num_calc(&value, 0, error))
-			return;
-
 		gchar buffer[TECO_EXPRESSIONS_FORMAT_LEN];
-		gchar *str = teco_expressions_format(buffer, value,
+		gchar *str = teco_expressions_format(buffer,
+		                                     teco_expressions_pop_num(0),
 		                                     ctx->qreg_table_locals->radix);
 		g_assert(*str != '\0');
 		gsize len = strlen(str);
@@ -327,8 +323,7 @@ static void
 teco_state_start_loop_open(teco_machine_main_t *ctx, GError **error)
 {
 	teco_loop_context_t lctx;
-	if (!teco_expressions_eval(FALSE, error) ||
-	    !teco_expressions_pop_num_calc(&lctx.counter, -1, error))
+	if (!teco_expressions_pop_num_calc(&lctx.counter, -1, error))
 		return;
 	lctx.brace_level = teco_brace_level;
 	lctx.pass_through = teco_machine_main_eval_colon(ctx) > 0;
@@ -1203,8 +1198,7 @@ teco_state_condcommand_input(teco_machine_main_t *ctx, gunichar chr, GError **er
 			teco_error_argexpected_set(error, "\"");
 			return NULL;
 		}
-		if (!teco_expressions_pop_num_calc(&value, 0, error))
-			return NULL;
+		value = teco_expressions_pop_num(0);
 		break;
 
 	default:
@@ -1305,8 +1299,6 @@ TECO_DEFINE_STATE_COMMAND(teco_state_condcommand,
 static void
 teco_state_control_negate(teco_machine_main_t *ctx, GError **error)
 {
-	teco_int_t v;
-
 	if (!teco_expressions_eval(FALSE, error))
 		return;
 
@@ -1314,9 +1306,8 @@ teco_state_control_negate(teco_machine_main_t *ctx, GError **error)
 		teco_error_argexpected_set(error, "^_");
 		return;
 	}
-	if (!teco_expressions_pop_num_calc(&v, 0, error))
-		return;
-	teco_expressions_push(~v);
+
+	teco_expressions_push(~teco_expressions_pop_num(0));
 }
 
 static void
@@ -1390,9 +1381,8 @@ teco_state_control_radix(teco_machine_main_t *ctx, GError **error)
 			return;
 		teco_expressions_push(radix);
 	} else {
-		if (!teco_expressions_pop_num_calc(&radix, 0, error) ||
-		    !qreg->vtable->undo_set_integer(qreg, error) ||
-		    !qreg->vtable->set_integer(qreg, radix, error))
+		if (!qreg->vtable->undo_set_integer(qreg, error) ||
+		    !qreg->vtable->set_integer(qreg, teco_expressions_pop_num(0), error))
 			return;
 	}
 }
@@ -1444,9 +1434,7 @@ teco_state_control_glyphs2bytes(teco_machine_main_t *ctx, GError **error)
 		 */
 		res = teco_interface_ssm(colon_modified ? SCI_GETLENGTH : SCI_GETCURRENTPOS, 0, 0);
 	} else {
-		teco_int_t pos;
-		if (!teco_expressions_pop_num_calc(&pos, 0, error))
-			return;
+		teco_int_t pos = teco_expressions_pop_num(0);
 		if (colon_modified) {
 			/* teco_interface_bytes2glyphs() does not check addresses */
 			res = 0 <= pos && pos <= teco_interface_ssm(SCI_GETLENGTH, 0, 0)
@@ -1540,12 +1528,16 @@ teco_state_control_last_length(teco_machine_main_t *ctx, GError **error)
 	/*
 	 * There is little use in supporting n^S for n != 0.
 	 * This is just for consistency with ^Y.
+	 *
+	 * We do not use teco_expressions_pop_num_calc(),
+	 * so as not to reset the sign prefix.
 	 */
-	if (teco_expressions_args() > 0 &&
-	    !teco_expressions_pop_num_calc(&n, 0, error))
+	if (!teco_expressions_eval(FALSE, error))
 		return;
+	if (teco_expressions_args() > 0)
+		n = teco_expressions_pop_num(0);
 	if (n < 0 || n >= teco_ranges_count) {
-		teco_error_subpattern_set(error, "^Y");
+		teco_error_subpattern_set(error, "^S");
 		return;
 	}
 
@@ -2141,17 +2133,14 @@ teco_state_ecommand_properties(teco_machine_main_t *ctx, GError **error)
 	static teco_int_t caret_x = 0;
 
 	teco_int_t property;
-	if (!teco_expressions_eval(FALSE, error) ||
-	    !teco_expressions_pop_num_calc(&property, teco_num_sign, error))
+	if (!teco_expressions_pop_num_calc(&property, teco_num_sign, error))
 		return;
 
 	if (teco_expressions_args() > 0) {
 		/*
 		 * Set property
 		 */
-		teco_int_t value, color;
-		if (!teco_expressions_pop_num_calc(&value, 0, error))
-			return;
+		teco_int_t value = teco_expressions_pop_num(0);
 
 		switch (property) {
 		case EJ_MEMORY_LIMIT:
@@ -2170,9 +2159,8 @@ teco_state_ecommand_properties(teco_machine_main_t *ctx, GError **error)
 				teco_error_argexpected_set(error, "EJ");
 				return;
 			}
-			if (!teco_expressions_pop_num_calc(&color, 0, error))
-				return;
-			teco_interface_init_color((guint)value, (guint32)color);
+			teco_interface_init_color((guint)value,
+			                          (guint32)teco_expressions_pop_num(0));
 			break;
 
 		case EJ_CARETX:
@@ -2293,11 +2281,7 @@ teco_state_ecommand_eol(teco_machine_main_t *ctx, GError **error)
 		teco_int_t eol_mode;
 
 		if (teco_machine_main_eval_colon(ctx) > 0) {
-			teco_int_t v1, v2;
-			if (!teco_expressions_pop_num_calc(&v1, 0, error))
-				return;
-
-			switch (v1) {
+			switch (teco_expressions_pop_num(0)) {
 			case '\r':
 				eol_mode = SC_EOL_CR;
 				break;
@@ -2306,9 +2290,7 @@ teco_state_ecommand_eol(teco_machine_main_t *ctx, GError **error)
 					eol_mode = SC_EOL_LF;
 					break;
 				}
-				if (!teco_expressions_pop_num_calc(&v2, 0, error))
-					return;
-				if (v2 == '\r') {
+				if (teco_expressions_pop_num(0) == '\r') {
 					eol_mode = SC_EOL_CRLF;
 					break;
 				}
@@ -2319,8 +2301,7 @@ teco_state_ecommand_eol(teco_machine_main_t *ctx, GError **error)
 				return;
 			}
 		} else {
-			if (!teco_expressions_pop_num_calc(&eol_mode, 0, error))
-				return;
+			eol_mode = teco_expressions_pop_num(0);
 			switch (eol_mode) {
 			case SC_EOL_CRLF:
 			case SC_EOL_CR:
@@ -2469,10 +2450,7 @@ teco_state_ecommand_encoding(teco_machine_main_t *ctx, GError **error)
 	/*
 	 * Set code page
 	 */
-	teco_int_t new_cp;
-	if (!teco_expressions_pop_num_calc(&new_cp, 0, error))
-		return;
-
+	teco_int_t new_cp = teco_expressions_pop_num(0);
 	if (old_cp == SC_CP_UTF8 && new_cp == SC_CP_UTF8)
 		return;
 
@@ -2784,9 +2762,8 @@ teco_state_insert_initial(teco_machine_main_t *ctx, GError **error)
 		undo__teco_interface_ssm(SCI_UNDO, 0, 0);
 
 	/* This is done only now because it can _theoretically_ fail. */
-	for (gint i = args; i > 0; i--)
-		if (!teco_expressions_pop_num_calc(NULL, 0, error))
-			return FALSE;
+	for (gint i = 0; i < args; i++)
+		teco_expressions_pop_num(0);
 
 	return TRUE;
 }
diff --git a/src/expressions.c b/src/expressions.c
index ede54d2..2fbb659 100644
--- a/src/expressions.c
+++ b/src/expressions.c
@@ -76,12 +76,22 @@ teco_expressions_push_int(teco_int_t number)
 	undo__remove_index__teco_numbers(teco_numbers->len-1);
 }
 
+/** Peek into the numbers stack */
 teco_int_t
 teco_expressions_peek_num(guint index)
 {
 	return g_array_index(teco_numbers, teco_int_t, teco_numbers->len - 1 - index);
 }
 
+/**
+ * Pop a value from the number stack.
+ *
+ * This must only be called if you are sure that the number at the
+ * given index exists and is an argument, i.e. only after calling
+ * teco_expressions_eval() and teco_expressions_args().
+ * If you are unsure or want to imply a value,
+ * use teco_expressions_pop_num_calc() instead.
+ */
 teco_int_t
 teco_expressions_pop_num(guint index)
 {
@@ -99,6 +109,17 @@ teco_expressions_pop_num(guint index)
 	return n;
 }
 
+/**
+ * Pop an argument from the number stack.
+ *
+ * This resolves operations and allows implying values
+ * if there isn't any argument on the stack.
+ *
+ * @param ret Where to store the number
+ * @param imply The fallback value if there is no argument
+ * @param error A GError
+ * @return FALSE if an error occurred
+ */
 gboolean
 teco_expressions_pop_num_calc(teco_int_t *ret, teco_int_t imply, GError **error)
 {
@@ -263,6 +284,13 @@ teco_expressions_calc(GError **error)
 	return TRUE;
 }
 
+/**
+ * Resolve all operations on the top of the stack.
+ *
+ * @param pop_brace If TRUE this also pops the "brace" operator.
+ * @param error A GError
+ * @return FALSE if an error occurred
+ */
 gboolean
 teco_expressions_eval(gboolean pop_brace, GError **error)
 {
@@ -287,6 +315,14 @@ teco_expressions_eval(gboolean pop_brace, GError **error)
 	return TRUE;
 }
 
+/**
+ * Get number of numeric arguments on the top of the stack.
+ *
+ * @fixme You must call teco_expressions_eval() to resolve operations
+ * before this gives sensitive results.
+ * Overall it might be better to automatically call teco_expressions_eval()
+ * here or introduce a separate teco_expressions_args_calc().
+ */
 guint
 teco_expressions_args(void)
 {
diff --git a/src/glob.c b/src/glob.c
index af66951..9621f1a 100644
--- a/src/glob.c
+++ b/src/glob.c
@@ -470,8 +470,7 @@ teco_state_glob_filename_done(teco_machine_main_t *ctx, const teco_string_t *str
 
 	teco_int_t teco_test_mode;
 
-	if (!teco_expressions_eval(FALSE, error) ||
-	    !teco_expressions_pop_num_calc(&teco_test_mode, 0, error))
+	if (!teco_expressions_pop_num_calc(&teco_test_mode, 0, error))
 		return NULL;
 	switch (teco_test_mode) {
 	/*
diff --git a/src/move-commands.c b/src/move-commands.c
index 5c2159e..cf5d6b0 100644
--- a/src/move-commands.c
+++ b/src/move-commands.c
@@ -620,21 +620,16 @@ teco_state_start_delete_chars(teco_machine_main_t *ctx, GError **error)
 void
 teco_state_control_lines2glyphs(teco_machine_main_t *ctx, GError **error)
 {
-	if (!teco_expressions_eval(FALSE, error))
-		return;
-
 	if (teco_machine_main_eval_colon(ctx)) {
 		gssize pos;
 
+		if (!teco_expressions_eval(FALSE, error))
+			return;
+
 		if (!teco_expressions_args()) {
 			pos = teco_interface_ssm(SCI_GETCURRENTPOS, 0, 0);
 		} else {
-			teco_int_t v;
-
-			if (!teco_expressions_pop_num_calc(&v, 0, error))
-				return;
-
-			pos = teco_interface_glyphs2bytes(v);
+			pos = teco_interface_glyphs2bytes(teco_expressions_pop_num(0));
 			if (pos < 0) {
 				teco_error_range_set(error, "^Q");
 				return;
diff --git a/src/qreg-commands.c b/src/qreg-commands.c
index 5055058..bec2ca8 100644
--- a/src/qreg-commands.c
+++ b/src/qreg-commands.c
@@ -284,9 +284,7 @@ teco_state_queryqreg_got_register(teco_machine_main_t *ctx, teco_qreg_t *qreg,
 
 	if (teco_expressions_args() > 0) {
 		/* Query character from Q-Register string */
-		teco_int_t pos;
-		if (!teco_expressions_pop_num_calc(&pos, 0, error))
-			return NULL;
+		teco_int_t pos = teco_expressions_pop_num(0);
 		if (pos < 0) {
 			teco_error_range_set(error, "Q");
 			return NULL;
@@ -395,9 +393,7 @@ teco_state_setqregstring_nobuilding_done(teco_machine_main_t *ctx,
 			/* the glib docs wrongly claim that one character can take 6 bytes */
 			buffer = g_malloc(4*args);
 			for (gint i = args; i > 0; i--) {
-				teco_int_t v;
-				if (!teco_expressions_pop_num_calc(&v, 0, error))
-					return NULL;
+				teco_int_t v = teco_expressions_pop_num(0);
 				if (v < 0 || !g_unichar_validate(v)) {
 					teco_error_codepoint_set(error, "^U");
 					return NULL;
@@ -407,9 +403,7 @@ teco_state_setqregstring_nobuilding_done(teco_machine_main_t *ctx,
 		} else {
 			buffer = g_malloc(args);
 			for (gint i = args; i > 0; i--) {
-				teco_int_t v;
-				if (!teco_expressions_pop_num_calc(&v, 0, error))
-					return NULL;
+				teco_int_t v = teco_expressions_pop_num(0);
 				if (v < 0 || v > 0xFF) {
 					teco_error_codepoint_set(error, "^U");
 					return NULL;
diff --git a/src/ring.c b/src/ring.c
index 72aab93..d389e52 100644
--- a/src/ring.c
+++ b/src/ring.c
@@ -556,9 +556,7 @@ teco_state_save_file_done(teco_machine_main_t *ctx, const teco_string_t *str, GE
 	 */
 	teco_buffer_t *buffer = teco_ring_current;
 	if (teco_expressions_args() > 0) {
-		teco_int_t id;
-		if (!teco_expressions_pop_num_calc(&id, 0, error))
-			return NULL;
+		teco_int_t id = teco_expressions_pop_num(0);
 		buffer = teco_ring_find(id);
 		if (!buffer) {
 			teco_error_invalidbuf_set(error, id);
@@ -691,9 +689,7 @@ teco_state_ecommand_close(teco_machine_main_t *ctx, GError **error)
 	teco_buffer_t *buffer;
 	gboolean force;
 	if (teco_expressions_args() > 0) {
-		teco_int_t id;
-		if (!teco_expressions_pop_num_calc(&id, 0, error))
-			return;
+		teco_int_t id = teco_expressions_pop_num(0);
 		buffer = teco_ring_find(ABS(id));
 		if (!buffer) {
 			teco_error_invalidbuf_set(error, ABS(id));
diff --git a/src/search.c b/src/search.c
index 4724643..5ef2179 100644
--- a/src/search.c
+++ b/src/search.c
@@ -111,8 +111,7 @@ teco_state_search_initial(teco_machine_main_t *ctx, GError **error)
 		return FALSE;
 	if (teco_expressions_args()) {
 		/* TODO: optional count argument? */
-		if (!teco_expressions_pop_num_calc(&v1, 0, error))
-			return FALSE;
+		v1 = teco_expressions_pop_num(0);
 		if (v1 <= v2) {
 			teco_search_parameters.count = 1;
 			teco_search_parameters.from = teco_interface_glyphs2bytes(v1);
@@ -977,8 +976,7 @@ teco_state_search_all_initial(teco_machine_main_t *ctx, GError **error)
 		return FALSE;
 	if (teco_expressions_args()) {
 		/* TODO: optional count argument? */
-		if (!teco_expressions_pop_num_calc(&v1, 0, error))
-			return FALSE;
+		v1 = teco_expressions_pop_num(0);
 		if (v1 <= v2) {
 			teco_search_parameters.count = 1;
 			teco_search_parameters.from_buffer = teco_ring_find(v1);
diff --git a/src/spawn.c b/src/spawn.c
index abcce75..73f389e 100644
--- a/src/spawn.c
+++ b/src/spawn.c
@@ -204,7 +204,7 @@ teco_state_execute_initial(teco_machine_main_t *ctx, GError **error)
 		teco_int_t line;
 
 		teco_spawn_ctx.from = teco_interface_ssm(SCI_GETCURRENTPOS, 0, 0);
-		if (!teco_expressions_pop_num_calc(&line, 0, error))
+		if (!teco_expressions_pop_num_calc(&line, teco_num_sign, error))
 			return FALSE;
 		line += teco_interface_ssm(SCI_LINEFROMPOSITION, teco_spawn_ctx.from, 0);
 		teco_spawn_ctx.to = teco_interface_ssm(SCI_POSITIONFROMLINE, line, 0);
@@ -219,18 +219,13 @@ teco_state_execute_initial(teco_machine_main_t *ctx, GError **error)
 		break;
 	}
 
-	default: {
+	default:
 		/* pipe and replace character range */
-		teco_int_t from, to;
-		if (!teco_expressions_pop_num_calc(&to, 0, error) ||
-		    !teco_expressions_pop_num_calc(&from, 0, error))
-			return FALSE;
-		teco_spawn_ctx.from = teco_interface_glyphs2bytes(from);
-		teco_spawn_ctx.to = teco_interface_glyphs2bytes(to);
+		teco_spawn_ctx.to = teco_interface_glyphs2bytes(teco_expressions_pop_num(0));
+		teco_spawn_ctx.from = teco_interface_glyphs2bytes(teco_expressions_pop_num(0));
 		rc = teco_bool(teco_spawn_ctx.from <= teco_spawn_ctx.to &&
 		               teco_spawn_ctx.from >= 0 && teco_spawn_ctx.to >= 0);
 	}
-	}
 
 	if (teco_is_failure(rc)) {
 		if (!teco_machine_main_eval_colon(ctx)) {
diff --git a/src/symbols.c b/src/symbols.c
index 4028b7e..b5600e8 100644
--- a/src/symbols.c
+++ b/src/symbols.c
@@ -230,8 +230,6 @@ teco_state_scintilla_symbols_done(teco_machine_main_t *ctx, const teco_string_t
 	    !teco_expressions_eval(FALSE, error))
 		return NULL;
 
-	teco_int_t value;
-
 	if (!ctx->scintilla.iMessage) {
 		if (!teco_expressions_args()) {
 			g_set_error_literal(error, TECO_ERROR, TECO_ERROR_FAILED,
@@ -239,9 +237,7 @@ teco_state_scintilla_symbols_done(teco_machine_main_t *ctx, const teco_string_t
 			return NULL;
 		}
 
-		if (!teco_expressions_pop_num_calc(&value, 0, error))
-			return NULL;
-		ctx->scintilla.iMessage = value;
+		ctx->scintilla.iMessage = teco_expressions_pop_num(0);
 	}
 
 	return &teco_state_scintilla_lparam;
diff --git a/tests/testsuite.at b/tests/testsuite.at
index ae90b72..f24b58b 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -342,6 +342,8 @@ AT_SETUP([Execute external command])
 # So we have to wait until SciTECO supports a random number generator.
 TE_CHECK([[@EC'dd if=/dev/zero bs=512 count=1' Z= Z-512"N(0/0)']], 0, ignore, ignore)
 TE_CHECK([[0,128ED @EC'dd if=/dev/zero bs=512 count=1' Z= Z-512"N(0/0)']], 0, ignore, ignore)
+TE_CHECK([[@I/hello/ H@EC'tr a-z A-Z' J<0A"V(0/0)' :C;>]], 0, ignore, ignore)
+TE_CHECK([[@I/hello^J/ -@EC'tr a-z A-Z' J<0A"V(0/0)' :C;>]], 0, ignore, ignore)
 AT_CLEANUP
 
 AT_SETUP([Timestamps])