From: Matthew Fernandez Date: Sun, 18 Jul 2021 21:17:49 +0000 (-0700) Subject: rewrite edgepaint command line parsing using getopt_long X-Git-Tag: 2.49.0~50^2~3 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4bbc9c78fa8286df2ca7418336a56e3be2067fac;p=graphviz rewrite edgepaint command line parsing using getopt_long 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. --- diff --git a/CHANGELOG.md b/CHANGELOG.md index 437f82f9b..a49bf90ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/cmd/edgepaint/edgepaint.1 b/cmd/edgepaint/edgepaint.1 index bb0d790f2..593fb4933 100644 --- a/cmd/edgepaint/edgepaint.1 +++ b/cmd/edgepaint/edgepaint.1 @@ -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., diff --git a/cmd/edgepaint/edgepaintmain.c b/cmd/edgepaint/edgepaintmain.c index 6a68e09c8..7cc03c96c 100644 --- a/cmd/edgepaint/edgepaintmain.c +++ b/cmd/edgepaint/edgepaintmain.c @@ -12,6 +12,7 @@ #include #include +#include #include #include #include @@ -22,6 +23,9 @@ #include #include #include +#include +#include +#include typedef enum { FMT_GV, @@ -61,14 +65,14 @@ static void usage (char* cmd, int eval){ fprintf(stderr, "Usage: %s 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; + } } diff --git a/rtest/test_regression.py b/rtest/test_regression.py index 99e52c383..4891fdfc0 100644 --- a/rtest/test_regression.py +++ b/rtest/test_regression.py @@ -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