Add configuration file support.

[tkaden4]

This pull request references issue #74, adding configuration file support.

Configuration Files

Configuration files follow the same syntax as regular map expressions, except instead of using ; to indicate the end of an expression we use newlines, restricting it to one expression per line. Blank lines and lines starting with # are ignored. One catch to the parsing used is that the line must start with a # or whitespace to be recognized as a comment, and the # syntax cannot be used later in a line. This is an area of possible improvement moving forwards.

Argument Parsing Changes

A new argument [file ...] has been added, and has the following semantics:

• If a map expression is supplied via -e, it will be used in addition to files
• If files are supplied, they will be parsed, with - used for standard input
• If neither a map expression or files are supplied, then we use the default mapping from previous versions.

[alols]

This is great, not sure about reading from stdin though. I really don't see why anyone would want do that. It just adds confusion IMHO. "Do I use the -e option or stdin?"

[tkaden4]

The idea with that was to allow people to pipe in keymaps. I don't personally see it as confusing, but if I compare it to something like grep, I could see it being an odd feature. I'll remove it and add the -c option for configuration files.

[alols]

Awesome! Thanks for you contribution :)

[tkaden4]

I have updated all the necessary components and revised the man file and README. How does it look?

[c-max]

Very nice mod. I'm looking forward to use it.

Reading the code I might have found some problems:

1. Lines starting with # will be ignored (comment). So we won't be able to reference the modifier key by its keycode. But that's very useful if a 'modifier key' doesn't have a constant keysym. (My current mapping string starts with: #65=#255...)

2. read_line(): Won't the \r\n-handling concat these lines and lack termination by \0? (suggestion: case '\r': break;)

3. read_line(): If the last line of the config file doesn't end with \n or \0, it seems to be neither ignored nor terminated by \0 when returned by read_line().

4. line 648: line = realloc (line, nlen*sizeof(char));

[tkaden4]

The read_line function was the jankiest part of this code to write; Ill fix the issues you mentioned. How would -- work for comments instead? Also, the problem with line 648 isn't immediately apparent, what seems to be wrong?

[c-max]

You've got to be brave to process text (files) in C.

At the end it's up to you or alols to decide which char(s) will be used for comments (suspects: -- // ! ; ').

I'd use a single char because parsing is easier and editing is faster.

Line 648 lacks sizeof(char). I don't know if sizeof(char)==1 is true for all compilers. Since you wrote it in your other *lloc statements (e.g. line 616), you might want to write it there too.

[tkaden4]

I've added the sizeof(char) for consistency, thanks for pointing that out. @alols what do you think for comment characters? I personally like #, as most unix configuration files follow that format, but as pointed out by @c-max , that conflicts with the expression parser. I've used -- for now, as I consider ; to be for assembler comments and // for high level language comments.

[otommod]

The C standard does say that sizeof(char) == 1. You can find stackoverflow answers with the full references to the spec.

[tkaden4]

Even if that is true, it speaks to what the memory is being used for. I'll consider removing it if it's deemed more confusing. Thanks for letting me know, though.

[alols]

I think -- is fine for comments. While parsing text files in C is hard, reviewing it is even harder so thank you for helping out reviewing, @c-max !

[c-max]

Reading other's code is interesting because I learn a lot this way (methods, libs,...). And the typical German way to praise and admire nice things is to criticise - hoping to make them even nicer.

By making some test futher issues were found:

1. xcape.c:664:5: error: ‘for’ loop initial declarations are only allowed in C99 mode

2. Empty lines (pre trim) will make read_line() return NULL and terminate the while loop in parse_confs(). So a conf file made by echo -n -e "\nShift_L=l" will lead to Failed to parse_confs while echo -n -e " \nShift_L=l" (added space to avoid empty line) would work. IMHO line read_line() should return NULL if nlen == 0 AND (EOF or ERROR).

3. I still don't understand the idea of the case '\r' in read_line(). In the current version it ignores all \r. case '\r': break; would do the same much easier. But since something like echo -n -e "\rS\rh\ri\rf\rt_\rL\r=\rl" would create a valid conf, let me make an other suggestion: Let \r be handled by default and later in parse_confs() trim head and tail (isspace() will be true for \r). By doing so errors caused by trailing spaces can be avoided too. They would be hard to see/find.

char *trim(char *str)
{
    char *trimmed = str;
    char *end;

    if (str == NULL)
        return NULL;

    /* trim head */
    while(isspace((unsigned char)*trimmed)) trimmed++;

    if(*trimmed != '\0')
    {
        /* trim tail */
        end = trimmed + strlen(trimmed) - 1;
        while(isspace((unsigned char)*end)) end--;
        *(end+1) = '\0';
    }
    return trimmed;
}

[tkaden4]

Just want to make sure: is this library C89 compliant? If so, should we add it to the makefile?

[tkaden4]

The idea for handling \r was to allow \r\n files. I'll remove it and fix the empty line issue.