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]

I would prefer if we used an option prefix. Since -f is taken I suggest using -F. And an additional -F for every extra file.

[alols]

Another idea is to use -c for "config"

[alols]

Nice patch!

[alols]

Why not malloc?

[tkaden4]

Woops! I'll fix that right away.

[alols]

isn't this the same as current = current->next; ?

[tkaden4]

Not quite, since current is a double pointer, and -> only works on one level of indirection.

[alols]

I see. Could you please add some parenthesis to make it clearer.

[tkaden4]

done!

[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.