[PATCH] Make the parameter expansion subscript flags parse error message, "error in flags", identify the location of the parse error.

[PATCH] Make the parameter expansion subscript flags parse error message, "error in flags", identify the location of the parse error.

[Daniel Shahaf]

---
Review of the logic obtaining and massaging str_copy_for_output would be 
appreciated.  It's a 2500-line function and there are multiple char*
variables that refer to the same memory locations as *str, so it's
conceivable I overlooked something.

If it's too brittle, an alternative is to simply output «s» untokenized.
In the test case, that would output «x+):-}».

The offset is computed 1-based even when KSH_ARRAYS is set.  I don't
think interpretation of an error message should depend on the option
settings current at the time of the error message.

Cheers,

Daniel

 Src/subst.c            | 28 +++++++++++++++++++++++++---
 Test/D04parameter.ztst |  5 +++++
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/Src/subst.c b/Src/subst.c
index 8f5bd355e..96e0914eb 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -2259,9 +2259,31 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
 		    break;
 
 		default:
-		  flagerr:
-		    zerr("error in flags");
-		    return NULL;
+		flagerr:
+		    {
+			/* 
+			 * We're trying to output the string that failed to
+			 * parse and the offset of the parse error within that.
+			 *
+			 * The string is *str.  It hasn't been changed since
+			 * entry to this function, I think, except that the
+			 * first non-variable-declaration line in this function
+			 * (currently the 238th line in this function)
+			 * writes a NUL to the first place in *str, so we'll
+			 * compensate by outputting the dollar sign manually.
+			 */
+			char *str_copy_for_output = dupstring(*str + 1);
+
+			/* 
+			 * Convert to a 1-based offset, because the shell
+			 * language is 1-based by default.
+			 */
+			zlong offset = s - *str + 1;
+
+			untokenize(str_copy_for_output);
+			zerr("error in flags near position %z in '$%s'", offset, str_copy_for_output);
+			return NULL;
+		    }
 		}
 	    }
 	    s++;
diff --git a/Test/D04parameter.ztst b/Test/D04parameter.ztst
index ac99ff0e3..05bb61bdc 100644
--- a/Test/D04parameter.ztst
+++ b/Test/D04parameter.ztst
@@ -2691,3 +2691,8 @@ F:behavior, see http://austingroupbugs.net/view.php?id=888
   eval 'echo $(\*)'
 0:Backslash character other than newline is normal after $(
 >What a star
+
+ : ${(zZ+x+):-}
+1:parameter expansion flags parsing error gives a clue
+?(eval):1: error in flags near position 7 in '${(zZ+x+):-}'
+

Re: [PATCH] Make the parameter expansion subscript flags parse error message, "error in flags", identify the location of the parse error.

[Peter Stephenson]

On Sat, 2021-04-03 at 02:49 +0000, Daniel Shahaf wrote:
> ---
> Review of the logic obtaining and massaging str_copy_for_output would be 
> appreciated.  It's a 2500-line function and there are multiple char*
> variables that refer to the same memory locations as *str, so it's
> conceivable I overlooked something.

I think this is early enough in the function it should be fine.  It's
moving from left to right except where it needs to find something
matching, and the flags are first.

pws