Skip to content

Commit

Permalink
Improved MsgBox's smart comma handling.
Browse files Browse the repository at this point in the history 
  - % can now be used to make Options or Timeout an expression.

  - If the first arg is an expression, any unescaped comma which is not
    enclosed in quote marks or parentheses/brackets/braces will cause
    multi-arg mode to be used.  These commas were formerly interpreted
    as multi-statement operators within the first-and-only arg (Text).

  - When Title is an expression, unescaped commas contained within the
    expression no longer interfere with smart comma handling.

  - If there are exactly two args and the first is empty or an integer,
    multi-arg mode is used.  The former behaviour was to combine both
    into a single arg (Text).

  - Timeout can be a literal number or a single deref (and optionally
    part of a number; for example, %Timeout%.500).  Contrary to the
    documentation, the former behaviour interpreted most other cases
    beginning with '%' as expressions (containing a double-deref).

  - Title can be an expression even if Text and Options are omitted.
  • Loading branch information
@Lexikos
Lexikos committed Feb 4, 2012
1 parent 7ddcc85 commit 3d71fff
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 107 deletions.
2 changes: 1 addition & 1 deletion source/globaldata.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ Action g_act[] =
, {_T("IfNotExist"), 1, 1, 1, NULL} // File or directory.
// IfMsgBox must be physically adjacent to the other IFs in this array:
, {_T("IfMsgBox"), 1, 1, 1, NULL} // MsgBox result (e.g. OK, YES, NO)
, {_T("MsgBox"), 0, 4, 3, {4, 0}} // Text (if only 1 param) or: Mode-flag, Title, Text, Timeout.
, {_T("MsgBox"), 0, 4, 3, NULL} // Text (if only 1 param) or: Mode-flag, Title, Text, Timeout.
, {_T("InputBox"), 1, 11, 11 H, {5, 6, 7, 8, 10, 0}} // Output var, title, prompt, hide-text (e.g. passwords), width, height, X, Y, Font (e.g. courier:8 maybe), Timeout, Default
, {_T("SplashTextOn"), 0, 4, 4, {1, 2, 0}} // Width, height, title, text
, {_T("SplashTextOff"), 0, 0, 0, NULL}
Expand Down
165 changes: 59 additions & 106 deletions source/script.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4515,119 +4515,72 @@ ResultType Script::ParseAndAddLine(LPTSTR aLineText, ActionTypeType aActionType,
int mark, max_params_override = 0; // Set default.
if (aActionType == ACT_MSGBOX)
{
// First find out how many non-literal (non-escaped) delimiters are present.
// Use a high maximum so that we can almost always find and analyze the command's
// last apparent parameter. This helps error-checking be more informative in a
// case where the command specifies a timeout as its last param but it's next-to-last
// param contains delimiters that the user forgot to escape. In other words, this
// helps detect more often when the user is trying to use the timeout feature.
// If this weren't done, the command would more often forgive improper syntax
// and not report a load-time error, even though it's pretty obvious that a load-time
// error should have been reported:
#define MAX_MSGBOX_DELIMITERS 20
LPTSTR delimiter[MAX_MSGBOX_DELIMITERS];
int delimiter_count;
for (mark = delimiter_count = 0; action_args[mark] && delimiter_count < MAX_MSGBOX_DELIMITERS;)
for (int next, mark = 0, arg = 1; action_args[mark]; mark = next, ++arg)
{
for (; action_args[mark]; ++mark)
if (action_args[mark] == g_delimiter && !literal_map[mark]) // Match found: a non-literal delimiter.
{
delimiter[delimiter_count++] = action_args + mark;
++mark; // Skip over this delimiter for the next iteration of the outer loop.
if (arg > 1)
mark++; // Skip the delimiter...
while (IS_SPACE_OR_TAB(action_args[mark]))
mark++; // ...and any leading whitespace.

if (action_args[mark] == g_DerefChar && !literal_map[mark] && IS_SPACE_OR_TAB(action_args[mark+1]))
{
// Since this parameter is an expression, commas inside it can't be intended to be
// literal/displayed by the user unless they're enclosed in quotes; but in that case,
// the smartness below isn't needed because it's provided by the parameter-parsing
// logic in a later section.
if (arg >= 3) // Text or Timeout
break;
}
}
// If it has only 1 arg (i.e. 0 delimiters within the arg list) no override is needed.
// Otherwise do more checking:
if (delimiter_count)
{
LPTSTR cp;
// If the first apparent arg is not a non-blank pure number or there are apparently
// only 2 args present (i.e. 1 delimiter in the arg list), assume the command is being
// used in its 1-parameter mode:
if (delimiter_count <= 1) // 2 parameters or less.
// Force it to be 1-param mode. In other words, we want to make MsgBox a very forgiving
// command and have it rarely if ever report syntax errors:
max_params_override = 1;
else // It has more than 3 apparent params, but is the first param even numeric?
// Otherwise, just jump to the next parameter so we can check it too:
next = FindNextDelimiter(action_args, g_delimiter, mark+2, literal_map);
continue;
}

// Find the next non-literal delimiter:
for (next = mark; action_args[next]; ++next)
if (action_args[next] == g_delimiter && !literal_map[next])
break;

if (arg == 1) // Options (or Text in single-arg mode)
{
*delimiter[0] = '\0'; // Temporarily terminate action_args at the first delimiter.
if (!action_args[next]) // Below relies on this check.
break; // There's only one parameter, so no further checks are required.
// It has more than one apparent param, but is the first param even numeric?
action_args[next] = '\0'; // Temporarily terminate action_args at the first delimiter.
// Note: If it's a number inside a variable reference, it's still considered 1-parameter
// mode to avoid ambiguity (unlike the new deref checking for param #4 mentioned below,
// mode to avoid ambiguity (unlike the deref check for param #4 in the section below,
// there seems to be too much ambiguity in this case to justify trying to figure out
// if the first parameter is a pure deref, and thus that the command should use
// 3-param or 4-param mode instead).
if (!IsPureNumeric(action_args)) // No floats allowed. Allow all-whitespace for aut2 compatibility.
if (!IsPureNumeric(action_args + next)) // No floats allowed.
max_params_override = 1;
*delimiter[0] = g_delimiter; // Restore the string.
if (!max_params_override)
action_args[next] = g_delimiter; // Restore the string.
if (max_params_override)
break;
}
else if (arg == 4) // Timeout (or part of Text)
{
// Since the timeout feature is rarely used, if there are more than 4 parameters or
// exactly 4 but the 4th isn't blank, pure numeric, or a deref: assume it's being used
// in 3-parameter mode and that all the other delimiters were intended to be literal.

// If the 4th parameter isn't blank or pure numeric, assume the user didn't intend it
// to be the MsgBox timeout (since that feature is rarely used), instead intending it
// to be part of parameter #3.
if (!IsPureNumeric(action_args + mark, false, true, true))
{
// IMPORATANT: The MsgBox cmd effectively has 3 parameter modes:
// 1-parameter (where all commas in the 1st parameter are automatically literal)
// 3-parameter (where all commas in the 3rd parameter are automatically literal)
// 4-parameter (whether the 4th parameter is the timeout value)
// Thus, the below must be done in a way that recognizes & supports all 3 modes.
// The above has determined that the cmd isn't in 1-parameter mode.
// If at this point it has exactly 3 apparent params, allow the command to be
// processed normally without an override. Otherwise, do more checking:
if (delimiter_count == 3) // i.e. 3 delimiters, which means 4 params.
{
// If the 4th parameter isn't blank or pure numeric, assume the user didn't
// intend it to be the MsgBox timeout (since that feature is rarely used),
// instead intending it to be part of parameter #3.
if (!IsPureNumeric(delimiter[2] + 1, false, true, true))
{
// Not blank and not a int or float. Update for v1.0.20: Check if it's a
// single deref. If so, assume that deref contains the timeout and thus
// 4-param mode is in effect. This allows the timeout to be contained in
// a variable, which was requested by one user:
cp = omit_leading_whitespace(delimiter[2] + 1);
// Relies on short-circuit boolean order:
if (*cp != g_DerefChar || literal_map[cp - action_args]) // not a proper deref char.
max_params_override = 3;
// else since it does start with a real deref symbol, it must end with one otherwise
// that will be caught later on as a syntax error anyway. Therefore, don't override
// max_params, just let it be parsed as 4 parameters.
}
// If it has more than 4 params or it has exactly 4 but the 4th isn't blank,
// pure numeric, or a deref: assume it's being used in 3-parameter mode and
// that all the other delimiters were intended to be literal.
}
else if (delimiter_count > 3) // i.e. 4 or more delimiters, which means 5 or more params.
{
// v1.0.48: This section extends smart comma handling so that if parameter #3 (Text)
// is an expression, any commas in it won't interfere with the Timeout parameter.
// For example, the timeout parameter below should work now:
// MsgBox 0, Title, % Func(x,y), 1
//
// If the "Text" parameter is an expression then commas inside it can't be intended to
// be literal/displayed by the user unless they're enclosed in quotes; but in that case,
// the smartness below isn't needed because it's provided by the parameter-parsing logic
// in a later section. So in that case it seems safe to avoid setting max_params_override,
// which fixes examples like the one above. The code further below does this.
//
// By contrast, fixing the second parameter (Title) in a similar way would be more
// difficult and/or would be more likely to break existing scripts. For example if "title"
// is an expression but "text" is NOT an expression, there might be some commas in "text"
// that are currently handled as smart/auto-literal, and those cases should be preserved
// for backward compatibility.
//
// If expressions in the "title" parameter ever are fixed to not interfere with the
// Timeout parameter, perhaps the best way to do it would be to verify that it's an
// expression, then skip over any commas that are enclosed in quotes or parentheses so that
// the "real" commas can be counted and used by the rest of the smart-comma logic.
//
// For the section below, see comments at the similar code section above:
cp = omit_leading_whitespace(delimiter[1] + 1); // Parameter #3, the "text" parameter.
if ( *cp != g_DerefChar || literal_map[cp - action_args] // not a proper deref char...
|| !IS_SPACE_OR_TAB(cp[1]) ) // ...or it's not followed by a space or tab, so it isn't "% ".
// Since it has too many delimiters to be 4-param mode and since there is no "% "
// expression present, assume it's 3-param mode so that non-escaped commas in
// parameters 4 and beyond will be all treated as strings that are part of parameter #3.
max_params_override = 3;
}
//else if 3 params or less: Don't override via max_params_override, just parse it normally.
// Not blank and not a int or float. Update for v1.0.20: Check if it's a single
// deref. If so, assume that deref contains the timeout and thus 4-param mode is
// in effect. This allows the timeout to be contained in a variable, which was
// requested by one user. Update for v1.1.06: Do some additional checking to
// exclude things like "%x% marks the spot" but not "%Timeout%.500".
LPTSTR deref_end;
if (action_args[next] // There are too many delimiters (there appears to be another arg after this one).
|| action_args[mark] != g_DerefChar || literal_map[mark] // Not a proper deref char.
|| !(deref_end = _tcschr(action_args + mark + 1, g_DerefChar)) // No deref end char (this syntax error will be handled later).
|| !IsPureNumeric(deref_end + 1, false, true, true)) // There is something non-numeric following the deref (things like %Timeout%.500 are allowed for flexibility and backward-compatibility).
max_params_override = 3;
}
break;
}
}
} // end of special handling for MsgBox.
Expand Down Expand Up @@ -6869,11 +6822,11 @@ ResultType Script::AddLine(ActionTypeType aActionType, LPTSTR aArg[], int aArgc,

case ACT_MSGBOX:
if (aArgc > 1) // i.e. this MsgBox is using the 3-param or 4-param style.
if (!line.ArgHasDeref(1)) // i.e. if it's a deref, we won't try to validate it now.
if (!line.mArg[0].is_expression && !line.ArgHasDeref(1)) // i.e. if it's an expression (or an expression which was converted into a simple deref), we won't try to validate it now.
if (!IsPureNumeric(new_raw_arg1)) // Allow it to be entirely whitespace to indicate 0, like Aut2.
return ScriptError(ERR_PARAM1_INVALID, new_raw_arg1);
if (aArgc > 3) // EVEN THOUGH IT'S NUMERIC, due to MsgBox's smart-comma handling, this cannot be an expression because it would never have been detected as the fourth parameter to begin with.
if (!line.ArgHasDeref(4)) // i.e. if it's a deref, we won't try to validate it now.
if (aArgc > 3)
if (!line.mArg[3].is_expression && !line.ArgHasDeref(4)) // i.e. if it's an expression or deref, we won't try to validate it now.
if (!IsPureNumeric(new_raw_arg4, false, true, true))
return ScriptError(ERR_PARAM4_INVALID, new_raw_arg4);
break;
Expand Down

0 comments on commit 3d71fff

Please sign in to comment.