Re: [PATCH AMENDED] help: add "man.viewer" config var to use "woman" or "konqueror"

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 1, 2008, 02:14 UTC
Message-ID: <7vir079ong.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <200803010100.m2110ULs008889@localhost.localdomain>

Xavier Maillard <xma@gnu.org> writes:

Show 9 quoted lines

> This patch makes it possible to view man pages using other tools
> than the "man" program. It also implements support for emacs'
> "woman" and konqueror with the man KIO slave to view man pages.
>
> Note that "emacsclient" is used with option "-e" to launch "woman"
> on GNU emacs and this works only on versions >= 22.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> Amended-by: Xavier Maillard <xma@gnu.org>

When you do something like this, please CC the original author to keep him in the loop.

Show 22 quoted lines

> +static void exec_woman_emacs(const char *page)
> +{
> +	if (!check_emacsclient_version()) {
> +		/* This works only with emacsclient version >= 22. */
> +		struct strbuf man_page = STRBUF_INIT;
> +		strbuf_addf(&man_page, "(woman \"%s\")", page);
> +		execlp("emacsclient", "emacsclient", "-e", man_page.buf, NULL);
> +	} else // revert to old good man program
> +		execlp("man", "man", page, NULL);
> +}
> +
> +static void exec_man_konqueror(const char *page)
> +{
> +	const char *display = getenv("DISPLAY");
> +	if (display && *display) {
> +		struct strbuf man_page = STRBUF_INIT;
> +		strbuf_addf(&man_page, "man:%s(1)", page);
> +		execlp("kfmclient", "kfmclient", "newTab", man_page.buf, NULL);
> +	} else
> +		execlp("man", "man", page, NULL);
> +}
> +

I really do not think doing it this way would scale. What if somebody prefers konq under X, and otherwise woman but wants to fall back to man?

I do not think checking emacsclient version _every time_ is a good idea either, but for the sake of discussing an alternative approach, let's say that is fine.

How about allowing multi-valued man.viewer like this:

        [man]
                viewer = woman
                viewer = konqueror
                viewer = man

and have:

        static struct man_viewer {
                char *name;
                void (*exec)(const char *);
        } viewers[] = {
                { "woman", exec_woman },
                { "konqueror", exec_konqueror },
                { "man", exec_man },
                { NULL, },
        };

Then you can iterate the man.viewer values, ask the viewer's exec() function to show the page (or return when it is not in an environment that it can be useful).

show_man_page() would become:

	for (each viewer in user's config)
		viewer.exec(page); /* will return when unable */
	die("no man viewer handled the request");