Improve (or rather, rewrite) tbl(7) option parsing.
authorschwarze <schwarze@openbsd.org>
Mon, 26 Jan 2015 00:54:09 +0000 (00:54 +0000)
committerschwarze <schwarze@openbsd.org>
Mon, 26 Jan 2015 00:54:09 +0000 (00:54 +0000)
* Allow the layout to start after the semicolon on the options line.
* Ignore leading commas.
* Option arguments cannot contain closing parentheses.
* Avoid needless UNSUPP messages.
* Better ERROR reporting.
* Delete unused "linesize" field in struct tbl_opts.
* No need for static buffers.
* Garbage collect one almost empty wrapper function.
Improved functionality, but minus 40 lines of code.

regress/usr.bin/mandoc/tbl/Makefile
regress/usr.bin/mandoc/tbl/opt.in [new file with mode: 0644]
regress/usr.bin/mandoc/tbl/opt.out_ascii [new file with mode: 0644]
regress/usr.bin/mandoc/tbl/opt.out_lint [new file with mode: 0644]
usr.bin/mandoc/mandoc.h
usr.bin/mandoc/read.c
usr.bin/mandoc/tbl.c
usr.bin/mandoc/tbl_opts.c

index 270fcf9..83d48a2 100644 (file)
@@ -1,8 +1,8 @@
-# $OpenBSD: Makefile,v 1.7 2015/01/21 00:45:16 schwarze Exp $
+# $OpenBSD: Makefile,v 1.8 2015/01/26 00:54:09 schwarze Exp $
 
 REGRESS_TARGETS         = blankline center fonts macro misalign
-REGRESS_TARGETS        += nested numbers span vert
-LINT_TARGETS    = macro nested
+REGRESS_TARGETS        += nested numbers opt span vert
+LINT_TARGETS    = macro nested opt
 
 # groff-1.22.3 defect:
 # - When space is insufficient (on either side) for properly aligning
diff --git a/regress/usr.bin/mandoc/tbl/opt.in b/regress/usr.bin/mandoc/tbl/opt.in
new file mode 100644 (file)
index 0000000..e21a9b6
--- /dev/null
@@ -0,0 +1,19 @@
+.TH TBL-OPT 1 "January 25, 2015" OpenBSD
+.SH NAME
+tbl-opt \- table options
+.SH DESCRIPTION
+normal text
+.TS
+tab decimalpoint (,x) %foo box;
+n n .
+10.0   0.01
+0.01   10.0
+.TE
+.PP
+normal text
+.TS
+ , box,tab(:); l l .
+a:b
+.TE
+.PP
+normal text
diff --git a/regress/usr.bin/mandoc/tbl/opt.out_ascii b/regress/usr.bin/mandoc/tbl/opt.out_ascii
new file mode 100644 (file)
index 0000000..320b802
--- /dev/null
@@ -0,0 +1,24 @@
+TBL-OPT(1)                  General Commands Manual                 TBL-OPT(1)
+
+
+
+N\bNA\bAM\bME\bE
+       tbl-opt - table options
+
+D\bDE\bES\bSC\bCR\bRI\bIP\bPT\bTI\bIO\bON\bN
+       normal text
+
+       +--------------+
+       |10.0     0.01 |
+       | 0.01   10.0  |
+       +--------------+
+       normal text
+
+       +------+
+       |a   b |
+       +------+
+       normal text
+
+
+
+OpenBSD                        January 25, 2015                     TBL-OPT(1)
diff --git a/regress/usr.bin/mandoc/tbl/opt.out_lint b/regress/usr.bin/mandoc/tbl/opt.out_lint
new file mode 100644 (file)
index 0000000..85ba35c
--- /dev/null
@@ -0,0 +1,4 @@
+mandoc: opt.in:7:5: ERROR: missing tbl option argument: tab
+mandoc: opt.in:7:19: ERROR: wrong tbl option argument size: decimalpoint want 1 have 2
+mandoc: opt.in:7:23: ERROR: non-alphabetic character in tbl options: %
+mandoc: opt.in:7:24: ERROR: skipping unknown tbl option: foo
index 91696fd..23ccbfa 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: mandoc.h,v 1.128 2015/01/24 01:59:40 schwarze Exp $ */
+/*     $OpenBSD: mandoc.h,v 1.129 2015/01/26 00:54:09 schwarze Exp $ */
 /*
  * Copyright (c) 2010, 2011, 2014 Kristaps Dzonsons <kristaps@bsd.lv>
  * Copyright (c) 2010-2015 Ingo Schwarze <schwarze@openbsd.org>
@@ -136,6 +136,10 @@ enum       mandocerr {
        MANDOCERR_EQNEOF, /* unexpected end of equation */
 
        /* related to tables */
+       MANDOCERR_TBLOPT_ALPHA, /* non-alphabetic character in tbl options */
+       MANDOCERR_TBLOPT_BAD, /* skipping unknown tbl option: option */
+       MANDOCERR_TBLOPT_NOARG, /* missing tbl option argument */
+       MANDOCERR_TBLOPT_ARGSZ, /* wrong tbl option argument size */
        MANDOCERR_TBLNOLAYOUT, /* no table layout cells specified */
        MANDOCERR_TBLNODATA, /* no table data cells specified */
        MANDOCERR_TBLIGNDATA, /* ignore data in cell */
@@ -175,8 +179,6 @@ enum        mandocerr {
        MANDOCERR_TOOLARGE, /* input too large */
        MANDOCERR_CHAR_UNSUPP, /* unsupported control character: number */
        MANDOCERR_REQ_UNSUPP, /* unsupported roff request: request */
-       MANDOCERR_TBL, /* unsupported table syntax */
-       MANDOCERR_TBLOPT, /* unsupported table option */
        MANDOCERR_TBLLAYOUT, /* unsupported table layout */
        MANDOCERR_TBLMACRO, /* ignoring macro in table: macro */
 
@@ -186,7 +188,6 @@ enum        mandocerr {
 struct tbl_opts {
        char              tab; /* cell-separator */
        char              decimal; /* decimal point */
-       int               linesize;
        int               opts;
 #define        TBL_OPT_CENTRE   (1 << 0)
 #define        TBL_OPT_EXPAND   (1 << 1)
index 7f8c4c6..f2b4d96 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: read.c,v 1.90 2015/01/24 01:59:40 schwarze Exp $ */
+/*     $OpenBSD: read.c,v 1.91 2015/01/26 00:54:09 schwarze Exp $ */
 /*
  * Copyright (c) 2008, 2009, 2010, 2011 Kristaps Dzonsons <kristaps@bsd.lv>
  * Copyright (c) 2010-2015 Ingo Schwarze <schwarze@openbsd.org>
@@ -175,6 +175,10 @@ static     const char * const      mandocerrs[MANDOCERR_MAX] = {
        "unexpected end of equation",
 
        /* related to tables */
+       "non-alphabetic character in tbl options",
+       "skipping unknown tbl option",
+       "missing tbl option argument",
+       "wrong tbl option argument size",
        "no table layout cells specified",
        "no table data cells specified",
        "ignore data in cell",
@@ -213,8 +217,6 @@ static      const char * const      mandocerrs[MANDOCERR_MAX] = {
        "input too large",
        "unsupported control character",
        "unsupported roff request",
-       "unsupported table syntax",
-       "unsupported table option",
        "unsupported table layout",
        "ignoring macro in table",
 };
index ab6667c..16b4a40 100644 (file)
@@ -1,7 +1,7 @@
-/*     $OpenBSD: tbl.c,v 1.13 2015/01/21 00:45:16 schwarze Exp $ */
+/*     $OpenBSD: tbl.c,v 1.14 2015/01/26 00:54:09 schwarze Exp $ */
 /*
  * Copyright (c) 2009, 2010, 2011 Kristaps Dzonsons <kristaps@bsd.lv>
- * Copyright (c) 2011 Ingo Schwarze <schwarze@openbsd.org>
+ * Copyright (c) 2011, 2015 Ingo Schwarze <schwarze@openbsd.org>
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
 enum rofferr
 tbl_read(struct tbl_node *tbl, int ln, const char *p, int offs)
 {
-       int              len;
        const char      *cp;
-
-       cp = &p[offs];
-       len = (int)strlen(cp);
+       int              active;
 
        /*
-        * If we're in the options section and we don't have a
-        * terminating semicolon, assume we've moved directly into the
-        * layout section.  No need to report a warning: this is,
-        * apparently, standard behaviour.
+        * In the options section, proceed to the layout section
+        * after a semicolon, or right away if there is no semicolon.
+        * Ignore semicolons in arguments.
         */
 
-       if (TBL_PART_OPTS == tbl->part && len)
-               if (';' != cp[len - 1])
-                       tbl->part = TBL_PART_LAYOUT;
+       if (tbl->part == TBL_PART_OPTS) {
+               tbl->part = TBL_PART_LAYOUT;
+               active = 1;
+               for (cp = p; *cp != '\0'; cp++) {
+                       switch (*cp) {
+                       case '(':
+                               active = 0;
+                               continue;
+                       case ')':
+                               active = 1;
+                               continue;
+                       case ';':
+                               if (active)
+                                       break;
+                               continue;
+                       default:
+                               continue;
+                       }
+                       break;
+               }
+               if (*cp == ';') {
+                       tbl_option(tbl, ln, p);
+                       if (*(p = cp + 1) == '\0')
+                               return(ROFF_IGN);
+               }
+       }
 
-       /* Now process each logical section of the table.  */
+       /* Process the other section types.  */
 
        switch (tbl->part) {
-       case TBL_PART_OPTS:
-               tbl_option(tbl, ln, p);
-               return(ROFF_IGN);
        case TBL_PART_LAYOUT:
                tbl_layout(tbl, ln, p);
                return(ROFF_IGN);
@@ -79,7 +95,6 @@ tbl_alloc(int pos, int line, struct mparse *parse)
        tbl->parse = parse;
        tbl->part = TBL_PART_OPTS;
        tbl->opts.tab = '\t';
-       tbl->opts.linesize = 12;
        tbl->opts.decimal = '.';
        return(tbl);
 }
index 04fb25a..1593e7a 100644 (file)
@@ -1,6 +1,7 @@
-/*     $OpenBSD: tbl_opts.c,v 1.8 2015/01/14 22:44:51 schwarze Exp $ */
+/*     $OpenBSD: tbl_opts.c,v 1.9 2015/01/26 00:54:09 schwarze Exp $ */
 /*
  * Copyright (c) 2009, 2010, 2011 Kristaps Dzonsons <kristaps@bsd.lv>
+ * Copyright (c) 2015 Ingo Schwarze <schwarze@openbsd.org>
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -51,12 +52,6 @@ struct       tbl_phrase {
 /* Handle Commonwealth/American spellings. */
 #define        KEY_MAXKEYS      14
 
-/* Maximum length of key name string. */
-#define        KEY_MAXNAME      13
-
-/* Maximum length of key number size. */
-#define        KEY_MAXNUMSZ     10
-
 static const struct tbl_phrase keys[KEY_MAXKEYS] = {
        { "center",      TBL_OPT_CENTRE,        KEY_CENTRE},
        { "centre",      TBL_OPT_CENTRE,        KEY_CENTRE},
@@ -74,193 +69,119 @@ static    const struct tbl_phrase keys[KEY_MAXKEYS] = {
        { "nospaces",    TBL_OPT_NOSPACE,       KEY_NOSPACE},
 };
 
-static int              arg(struct tbl_node *, int,
+static void             arg(struct tbl_node *, int,
                                const char *, int *, enum tbl_ident);
-static void             opt(struct tbl_node *, int,
-                               const char *, int *);
 
 
-static int
+static void
 arg(struct tbl_node *tbl, int ln, const char *p, int *pos, enum tbl_ident key)
 {
-       int              i;
-       char             buf[KEY_MAXNUMSZ];
+       const char      *optname;
+       int              len, want;
 
        while (isspace((unsigned char)p[*pos]))
                (*pos)++;
 
-       /* Arguments always begin with a parenthesis. */
+       /* Arguments are enclosed in parentheses. */
 
-       if ('(' != p[*pos]) {
-               mandoc_msg(MANDOCERR_TBL, tbl->parse,
-                   ln, *pos, NULL);
-               return(0);
+       len = 0;
+       if (p[*pos] == '(') {
+               (*pos)++;
+               while (p[*pos + len] != ')')
+                       len++;
        }
 
-       (*pos)++;
-
-       /*
-        * The arguments can be ANY value, so we can't just stop at the
-        * next close parenthesis (the argument can be a closed
-        * parenthesis itself).
-        */
-
        switch (key) {
        case KEY_DELIM:
-               if ('\0' == p[(*pos)++]) {
-                       mandoc_msg(MANDOCERR_TBL, tbl->parse,
-                           ln, *pos - 1, NULL);
-                       return(0);
-               }
-
-               if ('\0' == p[(*pos)++]) {
-                       mandoc_msg(MANDOCERR_TBL, tbl->parse,
-                           ln, *pos - 1, NULL);
-                       return(0);
-               }
+               optname = "delim";
+               want = 2;
                break;
        case KEY_TAB:
-               if ('\0' != (tbl->opts.tab = p[(*pos)++]))
-                       break;
-
-               mandoc_msg(MANDOCERR_TBL, tbl->parse,
-                   ln, *pos - 1, NULL);
-               return(0);
+               optname = "tab";
+               want = 1;
+               if (len == want)
+                       tbl->opts.tab = p[*pos];
+               break;
        case KEY_LINESIZE:
-               for (i = 0; i < KEY_MAXNUMSZ && p[*pos]; i++, (*pos)++) {
-                       buf[i] = p[*pos];
-                       if ( ! isdigit((unsigned char)buf[i]))
-                               break;
-               }
-
-               if (i < KEY_MAXNUMSZ) {
-                       buf[i] = '\0';
-                       tbl->opts.linesize = atoi(buf);
-                       break;
-               }
-
-               mandoc_msg(MANDOCERR_TBL, tbl->parse, ln, *pos, NULL);
-               return(0);
+               optname = "linesize";
+               want = 0;
+               break;
        case KEY_DPOINT:
-               if ('\0' != (tbl->opts.decimal = p[(*pos)++]))
-                       break;
-
-               mandoc_msg(MANDOCERR_TBL, tbl->parse,
-                   ln, *pos - 1, NULL);
-               return(0);
+               optname = "decimalpoint";
+               want = 1;
+               if (len == want)
+                       tbl->opts.decimal = p[*pos];
+               break;
        default:
                abort();
                /* NOTREACHED */
        }
 
-       /* End with a close parenthesis. */
+       if (len == 0)
+               mandoc_msg(MANDOCERR_TBLOPT_NOARG,
+                   tbl->parse, ln, *pos, optname);
+       else if (want && len != want)
+               mandoc_vmsg(MANDOCERR_TBLOPT_ARGSZ,
+                   tbl->parse, ln, *pos,
+                   "%s want %d have %d", optname, want, len);
 
-       if (')' == p[(*pos)++])
-               return(1);
-
-       mandoc_msg(MANDOCERR_TBL, tbl->parse, ln, *pos - 1, NULL);
-       return(0);
+       *pos += len;
+       if (p[*pos] == ')')
+               (*pos)++;
 }
 
-static void
-opt(struct tbl_node *tbl, int ln, const char *p, int *pos)
+/*
+ * Parse one line of options up to the semicolon.
+ * Each option can be preceded by blanks and/or commas,
+ * and some options are followed by arguments.
+ */
+void
+tbl_option(struct tbl_node *tbl, int ln, const char *p)
 {
-       int              i, sv;
-       char             buf[KEY_MAXNAME];
-
-       /*
-        * Parse individual options from the stream as surrounded by
-        * this goto.  Each pass through the routine parses out a single
-        * option and registers it.  Option arguments are processed in
-        * the arg() function.
-        */
-
-again: /*
-        * EBNF describing this section:
-        *
-        * options      ::= option_list [:space:]* [;][\n]
-        * option_list  ::= option option_tail
-        * option_tail  ::= [,:space:]+ option_list |
-        *              ::= epsilon
-        * option       ::= [:alpha:]+ args
-        * args         ::= [:space:]* [(] [:alpha:]+ [)]
-        */
+       int              i, pos, len;
 
-       while (isspace((unsigned char)p[*pos]))
-               (*pos)++;
-
-       /* Safe exit point. */
-
-       if (';' == p[*pos])
-               return;
-
-       /* Copy up to first non-alpha character. */
+       pos = 0;
+       for (;;) {
+               while (isspace((unsigned char)p[pos]) || p[pos] == ',')
+                       pos++;
 
-       for (sv = *pos, i = 0; i < KEY_MAXNAME; i++, (*pos)++) {
-               buf[i] = (char)tolower((unsigned char)p[*pos]);
-               if ( ! isalpha((unsigned char)buf[i]))
-                       break;
-       }
+               if (p[pos] == ';')
+                       return;
 
-       /* Exit if buffer is empty (or overrun). */
+               /* Parse one option name. */
 
-       if (KEY_MAXNAME == i || 0 == i) {
-               mandoc_msg(MANDOCERR_TBL, tbl->parse, ln, *pos, NULL);
-               return;
-       }
+               len = 0;
+               while (isalpha((unsigned char)p[pos + len]))
+                       len++;
 
-       buf[i] = '\0';
+               if (len == 0) {
+                       mandoc_vmsg(MANDOCERR_TBLOPT_ALPHA,
+                           tbl->parse, ln, pos, "%c", p[pos]);
+                       pos++;
+                       continue;
+               }
 
-       while (isspace((unsigned char)p[*pos]) || p[*pos] == ',')
-               (*pos)++;
+               /* Look up the option name. */
 
-       /*
-        * Look through all of the available keys to find one that
-        * matches the input.  FIXME: hashtable this.
-        */
+               i = 0;
+               while (i < KEY_MAXKEYS &&
+                   (strncasecmp(p + pos, keys[i].name, len) ||
+                    keys[i].name[len] != '\0'))
+                       i++;
 
-       for (i = 0; i < KEY_MAXKEYS; i++) {
-               if (strcmp(buf, keys[i].name))
+               if (i == KEY_MAXKEYS) {
+                       mandoc_vmsg(MANDOCERR_TBLOPT_BAD, tbl->parse,
+                           ln, pos, "%.*s", len, p + pos);
+                       pos += len;
                        continue;
+               }
 
-               /*
-                * Note: this is more difficult to recover from, as we
-                * can be anywhere in the option sequence and it's
-                * harder to jump to the next.  Meanwhile, just bail out
-                * of the sequence altogether.
-                */
+               /* Handle the option. */
 
+               pos += len;
                if (keys[i].key)
                        tbl->opts.opts |= keys[i].key;
-               else if ( ! arg(tbl, ln, p, pos, keys[i].ident))
-                       return;
-
-               break;
+               else
+                       arg(tbl, ln, p, &pos, keys[i].ident);
        }
-
-       /*
-        * Allow us to recover from bad options by continuing to another
-        * parse sequence.
-        */
-
-       if (KEY_MAXKEYS == i)
-               mandoc_msg(MANDOCERR_TBLOPT, tbl->parse, ln, sv, NULL);
-
-       goto again;
-       /* NOTREACHED */
-}
-
-void
-tbl_option(struct tbl_node *tbl, int ln, const char *p)
-{
-       int              pos;
-
-       /*
-        * Table options are always on just one line, so automatically
-        * switch into the next input mode here.
-        */
-       tbl->part = TBL_PART_LAYOUT;
-
-       pos = 0;
-       opt(tbl, ln, p, &pos);
 }