]> granicus.if.org Git - graphviz/commitdiff
rewrite edgepaint command line parsing using getopt_long
authorMatthew Fernandez <matthew.fernandez@gmail.com>
Sun, 18 Jul 2021 21:17:49 +0000 (14:17 -0700)
committerMatthew Fernandez <matthew.fernandez@gmail.com>
Thu, 22 Jul 2021 00:18:37 +0000 (17:18 -0700)
This change makes edgepaint command line parsing code more standard and robust.
It now rejects invalid arguments and takes more standard GNU-style double dash
prefixed options. Note that the old style single dash prefixed options are also
still accepted for compatibility purposes. Closes #1971.

CHANGELOG.md
cmd/edgepaint/edgepaint.1
cmd/edgepaint/edgepaintmain.c
rtest/test_regression.py

index 437f82f9b83891db2860967a77b8ff1ac857f8b4..a49bf90ed0ebc776be9306adf45e3ca7aa0e3d56 100644 (file)
@@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   been updated to take `const char*` arguments #634
 - incorrectly using the `layout` attribute on anything other than a graph now
   results in a warning about this being invalid #2078
+- `edgepaint` accepts more standard `--` prefixed command line arguments and
+  rejects invalid options #1971
 
 ### Fixed
 
index bb0d790f24e1f17a27b842593ee19b3048881618..593fb4933bc2205b2cf19b5380f0da03ae4d548a 100644 (file)
@@ -21,23 +21,23 @@ them apart.
 .SH OPTIONS
 The following options are supported:
 .TP
-.BI \-accuracy=  e
+.BI \--accuracy=  e
 Accuracy with which to find the maximally different coloring for each node with
 regard to its neighbors. Default \fIe\fR = 0.01.
 .TP
-.BI \-angle= a
+.BI \--angle= a
 Color two edges differently if their incidence angle is less than \fIa\fR
 degrees. Default \fIa\fR = 15.
 .TP
-.BI \-random_seed= s
+.BI \--random_seed= s
 Random seed to use. \fIs\fR must be an integer. If \fIs\fR is negative, we do
 |\fIs\fR| iterations with different seeds and pick the best.
 .TP
-.BI \-lightness= "l1,l2"
+.BI \--lightness= "l1,l2"
 Only applies for the "lab" color scheme: \fIl1\fR and \fIl2\fR must integers,
 with 0 <= \fIl1\fR <= \fIl2\fR <=100. By default, we use "0,70".
 .TP
-.BI \-share_endpoint
+.BI \--share_endpoint
 If this option is specified, edges that share a node are not considered in
 conflict if they are close to parallel but are on the opposite sides of the node
 (around 180 degree).
@@ -45,7 +45,7 @@ conflict if they are close to parallel but are on the opposite sides of the node
 .BI \-o " f"
 Write output to file \fIf\fR (default: stdout).
 .TP
-.BI \-color_scheme= "c"
+.BI \--color_scheme= "c"
 Specifies the color scheme.  This can be "rgb", "gray", "lab" (default); or a
 comma-separated list of RGB colors in hex (e.g., "#ff0000,#aabbed,#eeffaa")
 representing a palette; or a string specifying a Brewer color scheme (e.g.,
index 6a68e09c8c0e4839a8018ebd57f37cc3610c1411..7cc03c96c7abb970be2f3275c8fdbedcadfd7906 100644 (file)
@@ -12,6 +12,7 @@
 
 #include <cgraph/cgraph.h>
 #include <cgraph/agxbuf.h>
+#include <cgraph/unreachable.h>
 #include <ingraphs/ingraphs.h>
 #include <common/pointset.h>
 #include <getopt.h>
@@ -22,6 +23,9 @@
 #include <edgepaint/node_distinct_coloring.h>
 #include <edgepaint/edge_distinct_coloring.h>
 #include <sparse/color_palette.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <string.h>
 
 typedef enum {
        FMT_GV,
@@ -61,14 +65,14 @@ static void usage (char* cmd, int eval){
   fprintf(stderr, "Usage: %s <options> gv file with 2D coordinates.\n", cmd);
   fprintf(stderr, "Find a color assignment of the edges, such that edges that cross at small angle have as different as possible.\n");
   fprintf(stderr, "Options are: \n");
-  fprintf(stderr, " -accuracy=e      : accuracy with which to find the maximally different coloring for each node with regard to its neighbors. Default 0.01.\n");
-  fprintf(stderr, " -angle=a         : if edge crossing is less than that angle a, then make the edge colors different. Default 15.\n");
-  fprintf(stderr, " -random_seed=s   : random seed to use. s must be an integer. If s is negative, we do -s iterations with different seeds and pick the best.\n");
-  fprintf(stderr, " -color_scheme=c  : palette used. The string c should be \"rgb\", \"gray\", \"lab\" (default); or\n");
+  fprintf(stderr, " --accuracy=e      : accuracy with which to find the maximally different coloring for each node with regard to its neighbors. Default 0.01.\n");
+  fprintf(stderr, " --angle=a         : if edge crossing is less than that angle a, then make the edge colors different. Default 15.\n");
+  fprintf(stderr, " --random_seed=s   : random seed to use. s must be an integer. If s is negative, we do -s iterations with different seeds and pick the best.\n");
+  fprintf(stderr, " --color_scheme=c  : palette used. The string c should be \"rgb\", \"gray\", \"lab\" (default); or\n");
   fprintf(stderr, "       a comma-separated list of RGB colors in hex (e.g., \"#ff0000,#aabbed,#eeffaa\"); or\n");
   fprintf(stderr, "       a string specifying a Brewer color scheme (e.g., \"accent7\"; see https://graphviz.org/doc/info/colors.html#brewer).\n");
-  fprintf(stderr, " -lightness=l1,l2 : only applied for LAB color scheme: l1 must be integer >=0, l2 integer <=100, and l1 <=l2. By default we use 0,70\n");
-  fprintf(stderr, " -share_endpoint  :  if this option is specified, edges that shares an end point are not considered in conflict if they are close to\n");
+  fprintf(stderr, " --lightness=l1,l2 : only applied for LAB color scheme: l1 must be integer >=0, l2 integer <=100, and l1 <=l2. By default we use 0,70\n");
+  fprintf(stderr, " --share_endpoint  :  if this option is specified, edges that shares an end point are not considered in conflict if they are close to\n");
   fprintf(stderr, "       parallel but is on the opposite ends of the shared point (around 180 degree).\n");
   fprintf(stderr, " -v               : verbose\n");
   fprintf(stderr, " -o fname         :  write output to file fname (stdout)\n");
@@ -98,10 +102,12 @@ checkG (Agraph_t* g)
        return 0;
 }
 
+static bool strprefix(const char *s1, const char *s2) {
+  return strncmp(s1, s2, strlen(s2)) == 0;
+}
 
 static void init(int argc, char *argv[], real *angle, real *accuracy, char **infile, int *check_edges_with_same_endpoint, int *seed, char **color_scheme, char **lightness){
 
-  int c;
   char* cmd = argv[0];
   outfile = NULL;
 
@@ -114,93 +120,149 @@ static void init(int argc, char *argv[], real *angle, real *accuracy, char **inf
   *color_scheme = "lab";
   *lightness = NULL;
 
-  while ((c = getopt(argc, argv, ":vc:a:s:r:l:o:?")) != -1) {
-    switch (c) {
-    case 's':
-      *check_edges_with_same_endpoint = 1;
+  while (true) {
+
+    // some constants above the range of valid ASCII to use as getopt markers
+    enum {
+      OPT_ACCURACY = 128,
+      OPT_ANGLE = 129,
+      OPT_COLOR_SCHEME = 130,
+      OPT_RANDOM_SEED = 131,
+      OPT_LIGHTNESS = 132,
+      OPT_SHARE_ENDPOINT = 133,
+    };
+
+    static const struct option opts[] = {
+        // clang-format off
+        {"accuracy",       required_argument, 0, OPT_ACCURACY},
+        {"angle",          required_argument, 0, OPT_ANGLE},
+        {"color_scheme",   required_argument, 0, OPT_COLOR_SCHEME},
+        {"random_seed",    required_argument, 0, OPT_RANDOM_SEED},
+        {"lightness",      required_argument, 0, OPT_LIGHTNESS},
+        {"share_endpoint", no_argument,       0, OPT_SHARE_ENDPOINT},
+        {0, 0, 0, 0},
+        // clang-format on
+    };
+
+    int option_index = 0;
+    int c = getopt_long(argc, argv, "a:c:r:l:o:s:v?", opts, &option_index);
+
+    if (c == -1) {
       break;
+    }
+
+    const char *arg = optarg;
+
+    // legacy handling of single-dash-prefixed options
+    if (c == 'a' && strprefix(arg, "ccuracy=")) {
+      c = OPT_ACCURACY;
+      arg += strlen("ccuracy=");
+    } else if (c == 'a' && strprefix(arg, "ngle=")) {
+      c = OPT_ANGLE;
+      arg += strlen("ngle=");
+    } else if (c == 'c' && strprefix(arg, "olor_scheme=")) {
+      c = OPT_COLOR_SCHEME;
+      arg += strlen("olor_scheme=");
+    } else if (c == 'r' && strprefix(arg, "andom_seed=")) {
+      c = OPT_RANDOM_SEED;
+      arg += strlen("andom_seed=");
+    } else if (c == 'l' && strprefix(arg, "ightness=")) {
+      c = OPT_LIGHTNESS;
+      arg += strlen("ightness=");
+    } else if (c == 's' && strprefix(arg, "hare_endpoint")) {
+      c = OPT_SHARE_ENDPOINT;
+    }
+
+    switch (c) {
+
+    // any valid use of these options should have been handled as legacy above
+    case 'a':
+    case 'c':
     case 'r':
-      if (strncmp(optarg,"andom_seed=", 11) == 0){
-       if (sscanf(optarg+11, "%d", seed) != 1){
-         fprintf(stderr,"-random_seed option must be a positive integer.\n");
-         usage(cmd, 1);
-       }
+    case 'l':
+      fprintf(stderr, "option -%c unrecognized.\n", c);
+      usage(cmd, EXIT_FAILURE);
+      UNREACHABLE();
+
+    case '?':
+      if (optopt == '\0' || optopt == '?') {
+        usage(cmd, EXIT_SUCCESS);
       }
-      break;
-    case 'a':
-      if (strncmp(optarg,"ccuracy=", 8) == 0){
-       sscanf(optarg+8, "%lf", accuracy);
-       if (*accuracy <= 0) {
-         fprintf(stderr,"-accuracy option must be a positive real number.\n");
-         usage(cmd, 1);
-       }
-      } else if (strncmp(optarg,"ngle=", 5) == 0){
-       sscanf(optarg+5, "%lf", angle);
-       if (*angle <= 0 || *angle >= 90) {
-         fprintf(stderr,"-angle option must be a positive real number between 0 to 90.\n");
-         usage(cmd, 1);
-       }
-      } else {
-       fprintf(stderr,"-accuracy option must contain a positive real.\n");
-       usage(cmd, 1);
+      fprintf(stderr, "option -%c unrecognized.\n", optopt);
+      usage(cmd, EXIT_FAILURE);
+      UNREACHABLE();
+
+    case 'o':
+      if (outfile != NULL) {
+        fclose(outfile);
       }
+      outfile = openFile(arg, "w", CmdName);
       break;
-    case 'c':
-      if (strncmp(optarg,"olor_scheme=", 12) == 0){
-       if (knownColorScheme(optarg + 12))
-          *color_scheme = optarg+12;
-        else {
-         fprintf(stderr,"-color_scheme option must be a valid string\n");
-         usage(cmd, 1);
-       }
-      } else {
-       usage(cmd, 1);
+
+    case 'v':
+      Verbose = TRUE;
+      break;
+
+    case OPT_ACCURACY:
+      if (sscanf(arg, "%lf", accuracy) != 1 || *accuracy <= 0) {
+        fprintf(stderr, "--accuracy option must be a positive real number.\n");
+        usage(cmd, EXIT_FAILURE);
       }
       break;
-    case 'l':{
-      int l1 = 0, l2 = 70;
-      if (strncmp(optarg,"ightness=", 9) == 0 && sscanf(optarg + 9, "%d,%d", &l1, &l2) == 2){
-       if (l1 < 0 || l2 > 100 || l1 > l2){
-         fprintf(stderr,"invalid -lightness=%s option.\n", optarg + 9);
-         usage(cmd, 1);
-       }
-       free(*lightness);
-       *lightness = malloc(sizeof(char)*10);
-       strcpy(*lightness, optarg + 9);
-      } else {
-       usage(cmd, 1);
+
+    case OPT_ANGLE:
+      if (sscanf(arg, "%lf", angle) != 1 || *angle <= 0 || *angle >= 90) {
+        fprintf(stderr, "--angle option must be a positive real number "
+                        "between 0 and 90.\n");
+        usage(cmd, EXIT_FAILURE);
       }
       break;
-    }
-    case 'v':
-      Verbose = TRUE;
+
+    case OPT_COLOR_SCHEME:
+      if (!knownColorScheme(arg)) {
+        fprintf(stderr,
+                "--color_scheme option must be a known color scheme.\n");
+        usage(cmd, EXIT_FAILURE);
+      }
       break;
-    case 'o':
-      if (outfile != NULL)
-        fclose(outfile);
-      outfile = openFile(optarg, "w", CmdName);
+
+    case OPT_LIGHTNESS: {
+      int l1 = 0;
+      int l2 = 70;
+      if (sscanf(arg, "%d,%d", &l1, &l2) != 2 || l1 < 0 || l2 > 100 ||
+          l1 > l2) {
+        fprintf(stderr, "invalid --lightness=%s option.\n", arg);
+        usage(cmd, EXIT_FAILURE);
+      }
+      free(*lightness);
+      *lightness = strdup(arg);
       break;
-    case '?':
-      if (optopt == '\0' || optopt == '?')
-       usage(cmd, 0);
-      else {
-       fprintf(stderr, "option -%c unrecognized\n",
-               optopt);
-       usage(cmd, 1);
+    }
+
+    case OPT_RANDOM_SEED:
+      if (sscanf(arg, "%d", seed) != 1) {
+        fprintf(stderr, "--random_seed option must be an integer.\n");
+        usage(cmd, EXIT_FAILURE);
       }
       break;
+
+    case OPT_SHARE_ENDPOINT:
+      *check_edges_with_same_endpoint = 1;
+      break;
+
+    default:
+      UNREACHABLE();
     }
   }
 
-  argv += optind;
-  argc -= optind;
-  
-  if (argc)
-    *infile = argv[0];
-
-  if (argc > 0) Files = argv;
-  if (!outfile) outfile = stdout;
+  if (argc > optind) {
+    *infile = argv[optind];
+    Files = argv + optind;
+  }
 
+  if (outfile == NULL) {
+    outfile = stdout;
+  }
 }
 
 
index 99e52c3833b962fbd5f6331418009633ebdacf31..4891fdfc00f8df5c3cb064832d4b7e5ea0af19f1 100644 (file)
@@ -1007,7 +1007,6 @@ def test_1931():
 
 @pytest.mark.skipif(shutil.which("edgepaint") is None,
                     reason="edgepaint not available")
-@pytest.mark.xfail(strict=True) # FIXME
 def test_1971():
   """
   edgepaint should reject invalid command line options