Re: the pager

So I set out to verify in the code that the order of priority of pager specification is

    GIT_PAGER > core.pager > PAGER > default

I discovered that there is also a pager.<command> configuration variable.

I was expecting the code to be simple, uniform (with regard to the 5 sources), and reasonably well documented. The relevant parts of the code that I have located so far include:

in environment.c:

    const char *pager_program;

in config.c:

    int git_config_with_options(config_fn_t fn, void *data,
                                const char *filename,
                                const char *blob_ref,
                                int respect_includes)
    {
            char *repo_config = NULL;
            int ret;
            struct config_include_data inc = CONFIG_INCLUDE_INIT;

            if (respect_includes) {
                    inc.fn = fn;
                    inc.data = data;
                    fn = git_config_include;
                    data = &inc;
            }

            /*
             * If we have a specific filename, use it. Otherwise, follow the
             * regular lookup sequence.
             */
            if (filename)
                    return git_config_from_file(fn, filename, data);
            else if (blob_ref)
                    return git_config_from_blob_ref(fn, blob_ref, data);

            repo_config = git_pathdup("config");
            ret = git_config_early(fn, data, repo_config);
            if (repo_config)
                    free(repo_config);
            return ret;
    }

in pager.c:

    /* returns 0 for "no pager", 1 for "use pager", and -1 for "not specified" */
    int check_pager_config(const char *cmd)
    {
            struct pager_config c;
            c.cmd = cmd;
            c.want = -1;
            c.value = NULL;
            git_config(pager_command_config, &c);
            if (c.value)
                    pager_program = c.value;
            return c.want;
    }

    const char *git_pager(int stdout_is_tty)
    {
            const char *pager;

            if (!stdout_is_tty)
                    return NULL;

            pager = getenv("GIT_PAGER");
            if (!pager) {
                    if (!pager_program)
                            git_config(git_default_config, NULL);
                    pager = pager_program;
            }
            if (!pager)
                    pager = getenv("PAGER");
            if (!pager)
                    pager = DEFAULT_PAGER;
            else if (!*pager || !strcmp(pager, "cat"))
                    pager = NULL;

            return pager;
    }

What's with the code? It's not simple, it's not uniform (e.g., setting env. var. PAGER to "cat" will cause git_pager() to return NULL, but setting preprocessor var. DEFAULT_PAGER to "cat" will cause it to return "cat"), and it's barely got any comments at all (a global variable has *no description whatsoever*).

I'd like to clean up the manual pages at least, but it would take me hours to figure out what the code *does*.

I know I'm griping here, but I thought that part of the reward for contributing to an open-source project was as a showcase of one's work. Commenting your code is what you learn first in programming.

Dale