LIST: Improve error messages.

diff --git a/src/language/data-io/list.c b/src/language/data-io/list.c
index 8a55f58a1230aa1cfa1a2865d6b987d5e9b8c1d0..6732e23a3dd89ed99f6dfe1d42d1e726a5ab9788 100644 (file)
--- a/src/language/data-io/list.c
+++ b/src/language/data-io/list.c
@@ -47,40 +47,28 @@
 #define N_(msgid) msgid
 #define _(msgid) gettext (msgid)
 
-enum numbering
+struct lst_cmd
   {
-    format_unnumbered,
-    format_numbered
+    long first;
+    long last;
+    long step;
+    const struct variable **vars;
+    size_t n_vars;
+    bool number_cases;
   };
 
-
-struct lst_cmd
-{
-  long first;
-  long last;
-  long step;
-  const struct variable **v_variables;
-  size_t n_variables;
-  enum numbering numbering;
-};
-
-
 static int
 list_execute (const struct lst_cmd *lcmd, struct dataset *ds)
 {
   const struct dictionary *dict = dataset_dict (ds);
 
-  bool ok;
-  int i;
-  struct casegrouper *grouper;
-  struct casereader *group;
   struct subcase sc;
-
   subcase_init_empty (&sc);
-  for (i = 0; i < lcmd->n_variables; i++)
-    subcase_add_var (&sc, lcmd->v_variables[i], SC_ASCEND);
-
+  for (size_t i = 0; i < lcmd->n_vars; i++)
+    subcase_add_var (&sc, lcmd->vars[i], SC_ASCEND);
 
+  struct casegrouper *grouper;
+  struct casereader *group;
   grouper = casegrouper_create_splits (proc_open (ds), dict);
   while (casegrouper_get_next_group (grouper, &group))
     {
@@ -101,13 +89,13 @@ list_execute (const struct lst_cmd *lcmd, struct dataset *ds)
 
       struct pivot_dimension *variables = pivot_dimension_create (
         table, PIVOT_AXIS_COLUMN, N_("Variables"));
-      for (int i = 0; i < lcmd->n_variables; i++)
+      for (size_t i = 0; i < lcmd->n_vars; i++)
         pivot_category_create_leaf (
-          variables->root, pivot_value_new_variable (lcmd->v_variables[i]));
+          variables->root, pivot_value_new_variable (lcmd->vars[i]));
 
       struct pivot_dimension *cases = pivot_dimension_create (
         table, PIVOT_AXIS_ROW, N_("Case Number"));
-      if (lcmd->numbering == format_numbered)
+      if (lcmd->number_cases)
         cases->root->show_label = true;
       else
         cases->hide_all_labels = true;
@@ -119,20 +107,21 @@ list_execute (const struct lst_cmd *lcmd, struct dataset *ds)
             cases->root, pivot_value_new_integer (case_num));
           case_num += lcmd->step;
 
-          for (int i = 0; i < lcmd->n_variables; i++)
+          for (size_t i = 0; i < lcmd->n_vars; i++)
             pivot_table_put2 (table, i, case_idx,
                               pivot_value_new_var_value (
-                                lcmd->v_variables[i], case_data_idx (c, i)));
+                                lcmd->vars[i], case_data_idx (c, i)));
         }
       casereader_destroy (group);
 
       pivot_table_submit (table);
     }
-  ok = casegrouper_destroy (grouper);
+
+  bool ok = casegrouper_destroy (grouper);
   ok = proc_commit (ds) && ok;
 
   subcase_uninit (&sc);
-  free (lcmd->v_variables);
+  free (lcmd->vars);
 
   return ok ? CMD_SUCCESS : CMD_CASCADING_FAILURE;
 }
@@ -142,17 +131,13 @@ list_execute (const struct lst_cmd *lcmd, struct dataset *ds)
 int
 cmd_list (struct lexer *lexer, struct dataset *ds)
 {
-  struct lst_cmd cmd;
   const struct dictionary *dict = dataset_dict (ds);
 
-  /* Fill in defaults. */
-  cmd.step = 1;
-  cmd.first = 1;
-  cmd.last = LONG_MAX;
-  cmd.n_variables = 0;
-  cmd.v_variables = NULL;
-  cmd.numbering = format_unnumbered;
-
+  struct lst_cmd cmd = {
+    .step = 1,
+    .first = 1,
+    .last = LONG_MAX,
+  };
 
   while (lex_token (lexer) != T_ENDCMD)
     {
@@ -160,99 +145,74 @@ cmd_list (struct lexer *lexer, struct dataset *ds)
       if (lex_match_id (lexer, "VARIABLES"))
         {
           lex_match (lexer, T_EQUALS);
-          if (! parse_variables_const (lexer, dict, &cmd.v_variables, &cmd.n_variables, 0))
-            {
-              msg (SE, _("No variables specified."));
-              return CMD_FAILURE;
-            }
+          free (cmd.vars);
+          cmd.vars = NULL;
+          if (!parse_variables_const (lexer, dict, &cmd.vars, &cmd.n_vars,
+                                      PV_DUPLICATE))
+            goto error;
         }
       else if (lex_match_id (lexer, "FORMAT"))
         {
           lex_match (lexer, T_EQUALS);
           if (lex_match_id (lexer, "NUMBERED"))
-            {
-              cmd.numbering = format_numbered;
-            }
+            cmd.number_cases = true;
           else if (lex_match_id (lexer, "UNNUMBERED"))
-            {
-              cmd.numbering = format_unnumbered;
-            }
+            cmd.number_cases = false;
           else
             {
-              lex_error (lexer, NULL);
+              lex_error_expecting (lexer, "NUMBERED", "UNNUMBERED");
               goto error;
             }
         }
-      /* example: LIST /CASES=FROM 1 TO 25 BY 5. */
       else if (lex_match_id (lexer, "CASES"))
         {
           lex_match (lexer, T_EQUALS);
-          if (lex_match_id (lexer, "FROM") && lex_force_int (lexer))
+
+          if (lex_match_id (lexer, "FROM"))
             {
+              if (!lex_force_int_range (lexer, "FROM", 1, LONG_MAX))
+                goto error;
               cmd.first = lex_integer (lexer);
               lex_get (lexer);
             }
+          else
+            cmd.first = 1;
 
-          if ((lex_match (lexer, T_TO) && lex_force_int (lexer))
-              || lex_is_integer (lexer))
+          if (lex_match (lexer, T_TO) || lex_is_integer (lexer))
             {
+              if (!lex_force_int_range (lexer, "TO", cmd.first, LONG_MAX))
+                goto error;
               cmd.last = lex_integer (lexer);
               lex_get (lexer);
             }
+          else
+            cmd.last = LONG_MAX;
 
-          if (lex_match (lexer, T_BY) && lex_force_int (lexer))
+          if (lex_match (lexer, T_BY))
             {
+              if (!lex_force_int_range (lexer, "TO", 1, LONG_MAX))
+                goto error;
               cmd.step = lex_integer (lexer);
               lex_get (lexer);
             }
+          else
+            cmd.step = 1;
         }
-      else if (! parse_variables_const (lexer, dict, &cmd.v_variables, &cmd.n_variables, 0))
+      else
         {
-          return CMD_FAILURE;
+          free (cmd.vars);
+          cmd.vars = NULL;
+          if (!parse_variables_const (lexer, dict, &cmd.vars, &cmd.n_vars,
+                                       PV_DUPLICATE))
+            goto error;
         }
     }
 
-
-  /* Verify arguments. */
-  if (cmd.first > cmd.last)
-    {
-      int t;
-      msg (SW, _("The first case (%ld) specified precedes the last case (%ld) "
-                 "specified.  The values will be swapped."), cmd.first, cmd.last);
-      t = cmd.first;
-      cmd.first = cmd.last;
-      cmd.last = t;
-    }
-
-  if (cmd.first < 1)
-    {
-      msg (SW, _("The first case (%ld) to list is numbered less than 1.  "
-                 "The value is being reset to 1."), cmd.first);
-      cmd.first = 1;
-    }
-
-  if (cmd.last < 1)
-    {
-      msg (SW, _("The last case (%ld) to list is numbered less than 1.  "
-                 "The value is being reset to 1."), cmd.last);
-      cmd.last = 1;
-    }
-
-  if (cmd.step < 1)
-    {
-      msg (SW, _("The step value %ld is less than 1.  The value is being "
-                 "reset to 1."), cmd.step);
-      cmd.step = 1;
-    }
-
-  /* If no variables were explicitly provided, then default to ALL */
-  if (cmd.n_variables == 0)
-    dict_get_vars (dict, &cmd.v_variables, &cmd.n_variables,
-                   DC_SYSTEM | DC_SCRATCH);
-
+  if (!cmd.n_vars)
+    dict_get_vars (dict, &cmd.vars, &cmd.n_vars, DC_SYSTEM | DC_SCRATCH);
   return list_execute (&cmd, ds);
 
  error:
-  free (cmd.v_variables);
+  free (cmd.vars);
   return CMD_FAILURE;
 }

diff --git a/tests/language/data-io/list.at b/tests/language/data-io/list.at
index 5dd3af42bcff989f0a096aaf19b65ab277c4578f..c10c95a80943f737603d5386d6a443baacfdea3d 100644 (file)
--- a/tests/language/data-io/list.at
+++ b/tests/language/data-io/list.at
@@ -325,3 +325,40 @@ Case Number,forename,height
 4,David,109.10
 ])
 AT_CLEANUP
+
+AT_SETUP([LIST syntax errors])
+AT_DATA([list.sps], [dnl
+DATA LIST LIST NOTABLE /x.
+LIST VARIABLES=**.
+LIST FORMAT=**.
+LIST CASES=FROM -1.
+LIST CASES=FROM 5 TO 4.
+LIST CASES=BY 0.
+LIST **.
+])
+AT_CHECK([pspp -O format=csv list.sps], [1], [dnl
+"list.sps:2.16-2.17: error: LIST: Syntax error expecting variable name.
+    2 | LIST VARIABLES=**.
+      |                ^~"
+
+"list.sps:3.13-3.14: error: LIST: Syntax error expecting NUMBERED or UNNUMBERED.
+    3 | LIST FORMAT=**.
+      |             ^~"
+
+"list.sps:4.17-4.18: error: LIST: Syntax error expecting positive integer for FROM.
+    4 | LIST CASES=FROM -1.
+      |                 ^~"
+
+"list.sps:5.22: error: LIST: Syntax error expecting integer 5 or greater for TO.
+    5 | LIST CASES=FROM 5 TO 4.
+      |                      ^"
+
+"list.sps:6.15: error: LIST: Syntax error expecting positive integer for TO.
+    6 | LIST CASES=BY 0.
+      |               ^"
+
+"list.sps:7.6-7.7: error: LIST: Syntax error expecting variable name.
+    7 | LIST **.
+      |      ^~"
+])
+AT_CLEANUP
\ No newline at end of file