Dzen is a general purpose messaging, notification and menuing program for X11 written by Robert Manea. In order to increase my experience with programming I have started rewriting portions of the source code. I started with the following 148 lines within the main function that parsed the command-line arguments.
/* cmdline args */
for(i = 1; i < argc; i++)
if(!strncmp(argv[i], "-l", 3)){
if(++i < argc) {
dzen.slave_win.max_lines = atoi(argv[i]);
if(dzen.slave_win.max_lines)
init_input_buffer();
}
}
else if(!strncmp(argv[i], "-geometry", 10)) {
if(++i < argc) {
int t;
int tx, ty;
unsigned int tw, th;
t = XParseGeometry(argv[i], &tx, &ty, &tw, &th);
if(t & XValue)
dzen.title_win.x = tx;
if(t & YValue) {
dzen.title_win.y = ty;
if(!ty && (t & YNegative))
/* -0 != +0 */
dzen.title_win.y = -1;
}
if(t & WidthValue)
dzen.title_win.width = (signed int) tw;
if(t & HeightValue)
dzen.line_height = (signed int) th;
}
}
else if(!strncmp(argv[i], "-u", 3)){
dzen.tsupdate = True;
}
else if(!strncmp(argv[i], "-expand", 8)){
if(++i < argc) {
switch(argv[i][0]){
case 'l':
dzen.title_win.expand = left;
break;
case 'c':
dzen.title_win.expand = both;
break;
case 'r':
dzen.title_win.expand = right;
break;
default:
dzen.title_win.expand = noexpand;
}
}
}
else if(!strncmp(argv[i], "-p", 3)) {
dzen.ispersistent = True;
if (i+1 < argc) {
dzen.timeout = strtoul(argv[i+1], &endptr, 10);
if(*endptr)
dzen.timeout = 0;
else
i++;
}
}
else if(!strncmp(argv[i], "-ta", 4)) {
if(++i < argc) dzen.title_win.alignment = alignment_from_char(argv[i][0]);
}
else if(!strncmp(argv[i], "-sa", 4)) {
if(++i < argc) dzen.slave_win.alignment = alignment_from_char(argv[i][0]);
}
else if(!strncmp(argv[i], "-m", 3)) {
dzen.slave_win.ismenu = True;
if(i+1 < argc) {
if( argv[i+1][0] == 'v') {
++i;
break;
}
dzen.slave_win.ishmenu = (argv[i+1][0] == 'h') ? ++i, True : False;
}
}
else if(!strncmp(argv[i], "-fn", 4)) {
if(++i < argc) dzen.fnt = argv[i];
}
else if(!strncmp(argv[i], "-e", 3)) {
if(++i < argc) action_string = argv[i];
}
else if(!strncmp(argv[i], "-title-name", 12)) {
if(++i < argc) dzen.title_win.name = argv[i];
}
else if(!strncmp(argv[i], "-slave-name", 12)) {
if(++i < argc) dzen.slave_win.name = argv[i];
}
else if(!strncmp(argv[i], "-bg", 4)) {
if(++i < argc) dzen.bg = argv[i];
}
else if(!strncmp(argv[i], "-fg", 4)) {
if(++i < argc) dzen.fg = argv[i];
}
else if(!strncmp(argv[i], "-x", 3)) {
if(++i < argc) dzen.title_win.x = dzen.slave_win.x = atoi(argv[i]);
}
else if(!strncmp(argv[i], "-y", 3)) {
if(++i < argc) dzen.title_win.y = atoi(argv[i]);
}
else if(!strncmp(argv[i], "-w", 3)) {
if(++i < argc) dzen.slave_win.width = atoi(argv[i]);
}
else if(!strncmp(argv[i], "-h", 3)) {
if(++i < argc) dzen.line_height= atoi(argv[i]);
}
else if(!strncmp(argv[i], "-tw", 4)) {
if(++i < argc) dzen.title_win.width = atoi(argv[i]);
}
else if(!strncmp(argv[i], "-fn-preload", 12)) {
if(++i < argc) {
fnpre = estrdup(argv[i]);
}
}
#ifdef DZEN_XINERAMA
else if(!strncmp(argv[i], "-xs", 4)) {
if(++i < argc) dzen.xinescreen = atoi(argv[i]);
}
#endif
else if(!strncmp(argv[i], "-dock", 6))
use_ewmh_dock = 1;
else if(!strncmp(argv[i], "-v", 3)) {
printf("dzen-"VERSION", (C)opyright 2007-2009 Robert Manea\n");
printf(
"Enabled optional features: "
#ifdef DZEN_XMP
" XPM "
#endif
#ifdef DZEN_XFT
" XFT"
#endif
#ifdef DZEN_XINERAMA
" XINERAMA "
#endif
"\n"
);
return EXIT_SUCCESS;
}
else
eprint("usage: dzen2 [-v] [-p [seconds]] [-m [v|h]] [-ta <l|c|r>] [-sa <l|c|r>]\n"
" [-x <pixel>] [-y <pixel>] [-w <pixel>] [-h <pixel>] [-tw <pixel>] [-u]\n"
" [-e <string>] [-l <lines>] [-fn <font>] [-bg <color>] [-fg <color>]\n"
" [-geometry <geometry string>] [-expand <left|right>] [-dock]\n"
" [-title-name <string>] [-slave-name <string>]\n"
#ifdef DZEN_XINERAMA
" [-xs <screen>]\n"
#endif
);Not only does this lead to an unnecessarily long function, but it also includes a great deal of repetitive code. To reduce the repetition, and hopefully decrease the likelihood of a introducing a bug when adding a flag, I set about writing a table driven approach (see: opt.c). The core function and data structure of this design are copied below:
struct option
{
char *name;
int len;
int has_arg; // 0 false, 1 true, 2 optional
void (*setter)(Dzen *, char *);
} static opts[] = {
{ "-l", 2, 1, set_lines },
{ "-geometry", 9, 1, set_geometry },
{ "-u", 2, 0, set_update },
{ "-expand", 7, 1, set_expand },
{ "-p", 2, 2, set_persist },
{ "-ta", 3, 1, set_title_align },
{ "-sa", 4, 1, set_slave_align },
{ "-m", 2, 2, set_menu },
{ "-fn", 3, 1, set_font },
{ "-e", 2, 1, set_event },
{ "-title-name", 11, 1, set_title_name },
{ "-slave-name", 11, 1, set_slave_name },
{ "-bg", 3, 1, set_bg },
{ "-fg", 2, 1, set_fg },
{ "-y", 2, 1, set_y },
{ "-x", 2, 1, set_x },
{ "-w", 2, 1, set_width },
{ "-h", 2, 1, set_height },
{ "-tw", 3, 1, set_title_width },
{ "-fn-preload", 11, 1, set_font_preload },
#ifdef DZEN_XINERAMA
{ "-xs", 4, 1, set_xin_screen },
#endif
{ "-dock", 6, 0, set_dock },
{ "-v", 3, 0, print_version },
{ NULL, 0, 0, NULL }
};
void parse_opts( int ac, char **av, Dzen *dzen )
/*
* Loop through the command line arguments, and then the built-in flags, calling
* the appropriate function
*/
{
int i, j;
for (i = 1; i < ac; i++) { // Check command line arguments
for (j = 0; opts[j].name != NULL; j++) { // Compare built-in options
if (!strncmp(av[i], opts[j].name, opts[j].len)) {
if (opts[j].has_arg == 1) { // Required argument
if (++i < ac) {
opts[j].setter(dzen, av[i]);
}
else {
fprintf(stderr, "Missing argument: %s\n", opts[j].name);
exit(EXIT_FAILURE);
}
}
else if (opts[j].has_arg == 0) { // Not required argument
opts[j].setter(dzen, NULL);
i++;
}
else if (opts[j].has_arg == 2) { // Optional argument
if (i + 1 > ac) // Last option, no argument
opts[j].setter(dzen, NULL);
else if (av[i + 1][0] == '-') // Followed by option
opts[j].setter(dzen, NULL);
else
opts[j].setter(dzen, av[++i]);
}
break;
}
}
}
}With a basic table driven implementation finished I am left with two
issues that are preventing me from moving forward. The first is whether
the level of branching in the parse_opts function is too
high.
The second is how to deal with set_font_preload and
set_event functions, which assign a command-line argument
to a static character pointer, so that they may be processed later. If I
move the command-line parsing functions from main.c into a
separate source file (i.e. opt.c)
in order to increase modularity, then I still need to pass the character
pointers used by set_font_preload and
set_event back to main.c, so that their
contents may be processed once the command-line parsing is complete.