git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 1/2] help: add "man.viewer" config var to use "woman" or "konqueror"
@ 2008-02-28  4:19 Christian Couder
  2008-02-29  2:00 ` Xavier Maillard
  2008-02-29  2:00 ` [PATCH 1/2] " Xavier Maillard
  0 siblings, 2 replies; 12+ messages in thread
From: Christian Couder @ 2008-02-28  4:19 UTC (permalink / raw)
  To: Junio Hamano, Pascal Obry, Xavier Maillard; +Cc: git

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 emacs and this works only on versions >= 22.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 help.c |   81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 80 insertions(+), 1 deletions(-)

	A few minor changes since the previous version:
	- remove an unused "size" variable,
	- use right style for multi line comments,
	- use (void) for function with no arguments,
	- use [PATCH ...] in email subject.

	It should apply on top of 'next'.

diff --git a/help.c b/help.c
index e57a50e..2cb152d 100644
--- a/help.c
+++ b/help.c
@@ -8,6 +8,9 @@
 #include "exec_cmd.h"
 #include "common-cmds.h"
 #include "parse-options.h"
+#include "run-command.h"
+
+static const char *man_viewer;
 
 enum help_format {
 	HELP_FORMAT_MAN,
@@ -50,6 +53,8 @@ static int git_help_config(const char *var, const char *value)
 		help_format = parse_help_format(value);
 		return 0;
 	}
+	if (!strcmp(var, "man.viewer"))
+		return git_config_string(&man_viewer, var, value);
 	return git_default_config(var, value);
 }
 
@@ -345,11 +350,85 @@ static void setup_man_path(void)
 	strbuf_release(&new_path);
 }
 
+static int check_emacsclient_version(void)
+{
+	struct strbuf buffer = STRBUF_INIT;
+	struct child_process ec_process;
+	const char *argv_ec[] = { "emacsclient", "--version", NULL };
+	int version;
+
+	/* emacsclient prints its version number on stderr */
+	memset(&ec_process, 0, sizeof(ec_process));
+	ec_process.argv = argv_ec;
+	ec_process.err = -1;
+	ec_process.stdout_to_stderr = 1;
+	if (start_command(&ec_process)) {
+		fprintf(stderr, "Failed to start emacsclient.\n");
+		return -1;
+	}
+	strbuf_read(&buffer, ec_process.err, 20);
+	close(ec_process.err);
+
+	/*
+	 * Don't bother checking return value, because "emacsclient --version"
+	 * seems to always exits with code 1.
+	 */
+	finish_command(&ec_process);
+
+	if (prefixcmp(buffer.buf, "emacsclient")) {
+		fprintf(stderr, "Failed to parse emacsclient version.\n");
+		strbuf_release(&buffer);
+		return -1;
+	}
+
+	strbuf_remove(&buffer, 0, strlen("emacsclient"));
+	version = atoi(buffer.buf);
+
+	if (version < 22) {
+		fprintf(stderr,
+			"emacsclient version '%d' too old (< 22).\n",
+			version);
+		strbuf_release(&buffer);
+		return -1;
+	}
+
+	strbuf_release(&buffer);
+	return 0;
+}
+
+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
+		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);
+}
+
 static void show_man_page(const char *git_cmd)
 {
 	const char *page = cmd_to_page(git_cmd);
 	setup_man_path();
-	execlp("man", "man", page, NULL);
+	if (!man_viewer || !strcmp(man_viewer, "man"))
+		execlp("man", "man", page, NULL);
+	if (!strcmp(man_viewer, "woman"))
+		exec_woman_emacs(page);
+	if (!strcmp(man_viewer, "konqueror"))
+		exec_man_konqueror(page);
+	die("'%s': unsupported man viewer.", man_viewer);
 }
 
 static void show_info_page(const char *git_cmd)
-- 
1.5.4.3.328.gcaed.dirty

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] help: add "man.viewer" config var to use "woman" or "konqueror"
  2008-02-28  4:19 [PATCH 1/2] help: add "man.viewer" config var to use "woman" or "konqueror" Christian Couder
@ 2008-02-29  2:00 ` Xavier Maillard
  2008-02-29  7:14   ` Christian Couder
  2008-02-29  2:00 ` [PATCH 1/2] " Xavier Maillard
  1 sibling, 1 reply; 12+ messages in thread
From: Xavier Maillard @ 2008-02-29  2:00 UTC (permalink / raw)
  To: Christian Couder; +Cc: junkio, pascal, nanako3, git

Hi,

   Note that "emacsclient" is used with option "-e" to launch "woman"
   on emacs and this works only on versions >= 22.

I tested and it did not want to work at first. I modified it a
little bit and now it works with GNU Emacs 23.0.60 (CVS TRUNK).
Dunno if emacsclient has changed between 22 and CVS but it seems
it is printing onto stdout now. I will have to check that.

I am just attaching the patch with my modifications. Please
comment it and feel free to adapt it with yours.

Notice the if(!&buffer) is there just for me. It could be removed
safely. Also note that I did not develop C for ages so apologize
for my approximations ;)


diff --git a/help.c b/help.c
index e57a50e..b939f8d 100644
--- a/help.c
+++ b/help.c
@@ -8,6 +8,9 @@
 #include "exec_cmd.h"
 #include "common-cmds.h"
 #include "parse-options.h"
+#include "run-command.h"
+
+static const char *man_viewer;
 
 enum help_format {
 	HELP_FORMAT_MAN,
@@ -50,6 +53,8 @@ static int git_help_config(const char *var, const char *value)
 		help_format = parse_help_format(value);
 		return 0;
 	}
+	if (!strcmp(var, "man.viewer"))
+		return git_config_string(&man_viewer, var, value);
 	return git_default_config(var, value);
 }
 
@@ -345,11 +350,90 @@ static void setup_man_path(void)
 	strbuf_release(&new_path);
 }
 
+static int check_emacsclient_version(void)
+{
+	struct strbuf buffer = STRBUF_INIT;
+	struct child_process ec_process;
+
+	const char *argv_ec[] = { "emacsclient", "--version", NULL };
+	int version;
+	size_t len;
+
+	/* emacsclient prints its version number on stderr */
+	memset(&ec_process, 0, sizeof(ec_process));
+	ec_process.argv = argv_ec;
+	ec_process.err = -1;
+	ec_process.out = -1;
+	ec_process.stdout_to_stderr = 0;
+
+	if (start_command(&ec_process))
+		return error("Failed to start emacsclient.");
+
+	len = strbuf_read(&buffer, ec_process.out, 20);
+	close(ec_process.out);
+
+	/*
+	 * Don't bother checking return value, because "emacsclient --version"
+	 * seems to always exits with code 1.
+	 */
+	finish_command(&ec_process);
+
+	if (!&buffer)
+		return;
+
+	if (!len || len < 0 || prefixcmp(buffer.buf, "emacsclient")) {
+		strbuf_release(&buffer);
+		return error("Failed to parse emacsclient version.");
+	}
+
+	strbuf_remove(&buffer, 0, strlen("emacsclient"));
+	version = atoi(buffer.buf);
+
+	if (version < 22) {
+		fprintf(stderr,
+			"emacsclient version '%d' too old (< 22).\n",
+			version);
+		strbuf_release(&buffer);
+		return -1;
+	}
+
+	strbuf_release(&buffer);
+	return 0;
+}
+
+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
+		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);
+}
+
 static void show_man_page(const char *git_cmd)
 {
 	const char *page = cmd_to_page(git_cmd);
 	setup_man_path();
-	execlp("man", "man", page, NULL);
+	if (!man_viewer || !strcmp(man_viewer, "man"))
+		execlp("man", "man", page, NULL);
+	if (!strcmp(man_viewer, "woman"))
+		exec_woman_emacs(page);
+	if (!strcmp(man_viewer, "konqueror"))
+		exec_man_konqueror(page);
+	die("'%s': unsupported man viewer.", man_viewer);
 }
 
 static void show_info_page(const char *git_cmd)




	Xavier
-- 
http://www.gnu.org
http://www.april.org
http://www.lolica.org

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] help: add "man.viewer" config var to use "woman" or "konqueror"
  2008-02-28  4:19 [PATCH 1/2] help: add "man.viewer" config var to use "woman" or "konqueror" Christian Couder
  2008-02-29  2:00 ` Xavier Maillard
@ 2008-02-29  2:00 ` Xavier Maillard
  1 sibling, 0 replies; 12+ messages in thread
From: Xavier Maillard @ 2008-02-29  2:00 UTC (permalink / raw)
  To: Christian Couder; +Cc: junkio, pascal, nanako3, git

Hi (again),

   Note that "emacsclient" is used with option "-e" to launch "woman"
   on emacs and this works only on versions >= 22.

Here are more clues concerning emacsclient I found out when
trying to understand why it worked for you with >=22 and not for
me with a 23.x version.

   +
   +	/* emacsclient prints its version number on stderr */

As commented into another post, here the version is displayed on
stdout which is confirmed by reading the source code:

/* Display a normal or error message.
   On Windows, use a message box if compiled as a Windows app.  */

      FILE *f = is_error ? stderr : stdout;
      fputs (msg, f);
      fflush (f);

is_error is a boolean parameter to the function message.

   +	/*
   +	 * Don't bother checking return value, because "emacsclient --version"
   +	 * seems to always exits with code 1.
   +	 */

Almost, in fact, in emacsclient.c, we can read

	case 'V':
	  message (FALSE, "emacsclient %s\n", VERSION);
	  exit (EXIT_SUCCESS);
	  break;

So it always returns 0.

Regards

	Xavier
-- 
http://www.gnu.org
http://www.april.org
http://www.lolica.org

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] help: add "man.viewer" config var to use "woman" or "konqueror"
  2008-02-29  2:00 ` Xavier Maillard
@ 2008-02-29  7:14   ` Christian Couder
  2008-02-29  7:46     ` Junio C Hamano
  2008-03-01  1:00     ` [PATCH AMENDED] " Xavier Maillard
  0 siblings, 2 replies; 12+ messages in thread
From: Christian Couder @ 2008-02-29  7:14 UTC (permalink / raw)
  To: Xavier Maillard; +Cc: junkio, pascal, nanako3, git

Hi,

Le vendredi 29 février 2008, Xavier Maillard a écrit :
> Hi,
>
>    Note that "emacsclient" is used with option "-e" to launch "woman"
>    on emacs and this works only on versions >= 22.
>
> I tested and it did not want to work at first. I modified it a
> little bit and now it works with GNU Emacs 23.0.60 (CVS TRUNK).
> Dunno if emacsclient has changed between 22 and CVS but it seems
> it is printing onto stdout now. I will have to check that.

In fact on my main development machine there is only Emacsclient 21 and it 
outputs its version on stderr and exits with code 1. Emacsclient 21 also 
does not accept "-e" option.

I just checked with Emacsclient 22 and it outputs its version on stdout and 
exits with code 0. But it accepts the "-e" option.

I will need to get Emacs 23 from CVS but from what you say I guess it works 
like Emacs 22. 

So my "check_emacsclient_version" function works only with emacsclient 21 
and may be earlier versions.

I used:

	/* emacsclient prints its version number on stderr */
	memset(&ec_process, 0, sizeof(ec_process));
	ec_process.argv = argv_ec;
	ec_process.err = -1;
	ec_process.stdout_to_stderr = 1;

I hoped the above line would redirect stdout to stderr and I could read both 
from "ec_process.err" below, but it seems it doesn't work like that. I also 
just checked using "ec_process.stdout_to_stderr = 0;" but it still doesn't 
work. 

	if (start_command(&ec_process)) {
		fprintf(stderr, "Failed to start emacsclient.\n");
		return -1;
	}
	strbuf_read(&buffer, ec_process.err, 20);
	close(ec_process.err);

I will be offline this week end but I hope I can debug this next week.

Thanks,
Christian.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] help: add "man.viewer" config var to use "woman" or "konqueror"
  2008-02-29  7:14   ` Christian Couder
@ 2008-02-29  7:46     ` Junio C Hamano
  2008-03-01  1:00       ` Xavier Maillard
  2008-03-01  1:00     ` [PATCH AMENDED] " Xavier Maillard
  1 sibling, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2008-02-29  7:46 UTC (permalink / raw)
  To: Christian Couder; +Cc: Xavier Maillard, pascal, nanako3, git

Looking at this exchange really makes me suspect that these
things should not be in C.  Every time Emacs updates we need
recompilation?  Yuck.

What was the reason we gave up calling out to a single generic
scripted wrapper that the users can just munge to suit their
tastes?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] help: add "man.viewer" config var to use "woman" or "konqueror"
  2008-02-29  7:46     ` Junio C Hamano
@ 2008-03-01  1:00       ` Xavier Maillard
  2008-03-03  7:20         ` Christian Couder
  0 siblings, 1 reply; 12+ messages in thread
From: Xavier Maillard @ 2008-03-01  1:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: chriscool, pascal, nanako3, git


   Looking at this exchange really makes me suspect that these
   things should not be in C.  Every time Emacs updates we need
   recompilation?  Yuck.

In fact, I am pretty confident that emacsclient has always
printed its version onto stdout so I do not think we need to
recompile every week :)

   What was the reason we gave up calling out to a single generic
   scripted wrapper that the users can just munge to suit their
   tastes?

Good question. Though I find chriscool's implementation nicely
thought. With the modifications I made, this works as expected
with either emacs22 and emacs23 (aka CVS). What's more, how a
user could have tested if emacsclient was provided in the right
version using a generic scripted wrapper ?

I guess, chriscool should provide a more consistent emacsclient
wrapper with support for GNU Emacs version >= 21. This won't be
an enormous amount of work to do it (I can help him if he wants
to but I do not want to steal the paternity of his idea).

	Xavier
-- 
http://www.gnu.org
http://www.april.org
http://www.lolica.org

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH AMENDED] help: add "man.viewer" config var to use "woman" or "konqueror"
  2008-02-29  7:14   ` Christian Couder
  2008-02-29  7:46     ` Junio C Hamano
@ 2008-03-01  1:00     ` Xavier Maillard
  2008-03-01  2:14       ` Junio C Hamano
  1 sibling, 1 reply; 12+ messages in thread
From: Xavier Maillard @ 2008-03-01  1:00 UTC (permalink / raw)
  To: git


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>
---
I have tested it with emacsclient 22.x and 23.x. Previous
versions won't work at the moment.

 help.c |   85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 84 insertions(+), 1 deletions(-)

diff --git a/help.c b/help.c
index e57a50e..eb138f0 100644
--- a/help.c
+++ b/help.c
@@ -8,6 +8,9 @@
 #include "exec_cmd.h"
 #include "common-cmds.h"
 #include "parse-options.h"
+#include "run-command.h"
+
+static const char *man_viewer;
 
 enum help_format {
 	HELP_FORMAT_MAN,
@@ -50,6 +53,8 @@ static int git_help_config(const char *var, const char *value)
 		help_format = parse_help_format(value);
 		return 0;
 	}
+	if (!strcmp(var, "man.viewer"))
+		return git_config_string(&man_viewer, var, value);
 	return git_default_config(var, value);
 }
 
@@ -345,11 +350,89 @@ static void setup_man_path(void)
 	strbuf_release(&new_path);
 }
 
+static int check_emacsclient_version(void)
+{
+	struct strbuf buffer = STRBUF_INIT;
+	struct child_process ec_process;
+
+	const char *argv_ec[] = { "emacsclient", "--version", NULL };
+	int version;
+	size_t len;
+
+	/* emacsclient prints its version number on stdout */
+	memset(&ec_process, 0, sizeof(ec_process));
+	ec_process.argv = argv_ec;
+	ec_process.out = -1;
+	ec_process.stdout_to_stderr = 0;
+
+	if (start_command(&ec_process))
+		return error("Failed to start emacsclient.");
+
+	len = strbuf_read(&buffer, ec_process.out, 20);
+	close(ec_process.out);
+
+	/*
+	 * Don't bother checking return value, because "emacsclient --version"
+	 * always exits with code 0.
+	 */
+	finish_command(&ec_process);
+
+	if (!&buffer)
+		return -1;
+
+	if (!len || len < 0 || prefixcmp(buffer.buf, "emacsclient")) {
+		strbuf_release(&buffer);
+		return error("Failed to parse emacsclient version.");
+	}
+
+	strbuf_remove(&buffer, 0, strlen("emacsclient"));
+	version = atoi(buffer.buf);
+
+	if (version < 22) {
+		fprintf(stderr,
+			"emacsclient version '%d' too old (< 22).\n",
+			version);
+		strbuf_release(&buffer);
+		return -1;
+	}
+
+	strbuf_release(&buffer);
+	return 0;
+}
+
+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);
+}
+
 static void show_man_page(const char *git_cmd)
 {
 	const char *page = cmd_to_page(git_cmd);
 	setup_man_path();
-	execlp("man", "man", page, NULL);
+	if (!man_viewer || !strcmp(man_viewer, "man"))
+		execlp("man", "man", page, NULL);
+	if (!strcmp(man_viewer, "woman"))
+		exec_woman_emacs(page);
+	if (!strcmp(man_viewer, "konqueror"))
+		exec_man_konqueror(page);
+	die("'%s': unsupported man viewer.", man_viewer);
 }
 
 static void show_info_page(const char *git_cmd)
-- 
1.5.4.3.343.gc696

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH AMENDED] help: add "man.viewer" config var to use "woman" or "konqueror"
  2008-03-01  1:00     ` [PATCH AMENDED] " Xavier Maillard
@ 2008-03-01  2:14       ` Junio C Hamano
  2008-03-03  7:38         ` Christian Couder
  0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2008-03-01  2:14 UTC (permalink / raw)
  To: Xavier Maillard; +Cc: git

Xavier Maillard <xma@gnu.org> writes:

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

> +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");


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] help: add "man.viewer" config var to use "woman" or "konqueror"
  2008-03-01  1:00       ` Xavier Maillard
@ 2008-03-03  7:20         ` Christian Couder
  0 siblings, 0 replies; 12+ messages in thread
From: Christian Couder @ 2008-03-03  7:20 UTC (permalink / raw)
  To: Xavier Maillard; +Cc: Junio C Hamano, pascal, nanako3, git

Le samedi 1 mars 2008, Xavier Maillard a écrit :
>    Looking at this exchange really makes me suspect that these
>    things should not be in C.  Every time Emacs updates we need
>    recompilation?  Yuck.
>
> In fact, I am pretty confident that emacsclient has always
> printed its version onto stdout so I do not think we need to
> recompile every week :)

On my Kubuntu Feisty development machine with the stock emacs 
21.4a+1-2ubuntu1.1 package I get:

$ emacsclient --version
emacsclient 21.4
$ emacsclient --version 2>/dev/null
$ emacsclient --version 1>/dev/null
emacsclient 21.4

And I don't think it specific to Kubuntu because I remenber getting the same 
with emacs21 (before installing emacs22) on another (Debian testing) 
machine.

>    What was the reason we gave up calling out to a single generic
>    scripted wrapper that the users can just munge to suit their
>    tastes?

I didn't gave up, I am just waiting for the implementation of a scripted 
wrapper in git-mergetool.sh, that is now in 'pu', to mature. Then I will 
steal it so we have the same for git help.

> Good question. Though I find chriscool's implementation nicely
> thought.

Thanks.

> With the modifications I made, this works as expected 
> with either emacs22 and emacs23 (aka CVS). What's more, how a
> user could have tested if emacsclient was provided in the right
> version using a generic scripted wrapper ?

I agree that it's better to do it for the users.

In fact just after this email, I will send a patch to "run_command.c" named:

"Redirect stderr to a pipe before redirecting stdout to stderr"

that will make my last patches to "help.c" work with both Emacs21 and 
Emacs22.

> I guess, chriscool should provide a more consistent emacsclient
> wrapper with support for GNU Emacs version >= 21. This won't be
> an enormous amount of work to do it (I can help him if he wants
> to but I do not want to steal the paternity of his idea).

It would help if you could test with Emacs23 as I didn't installed it.

Thank you for testing my patches.

Christian.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH AMENDED] help: add "man.viewer" config var to use "woman" or "konqueror"
  2008-03-01  2:14       ` Junio C Hamano
@ 2008-03-03  7:38         ` Christian Couder
  2008-03-03  7:42           ` Jakub Narebski
  0 siblings, 1 reply; 12+ messages in thread
From: Christian Couder @ 2008-03-03  7:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Xavier Maillard, git

Le samedi 1 mars 2008, Junio C Hamano a écrit :
> Xavier Maillard <xma@gnu.org> writes:
> > 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.

Yes please.

> 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

I tried the above in my .git/config and I get:

$ git config man.viewer
woman
error: More than one value for the key man.viewer: konqueror
error: More than one value for the key man.viewer: man

so I guess this can work only in C.

> 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");

Great idea, I will see if I can do that in some latter patches.

Thanks,
Christian.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH AMENDED] help: add "man.viewer" config var to use "woman" or "konqueror"
  2008-03-03  7:38         ` Christian Couder
@ 2008-03-03  7:42           ` Jakub Narebski
  2008-03-03 19:07             ` Christian Couder
  0 siblings, 1 reply; 12+ messages in thread
From: Jakub Narebski @ 2008-03-03  7:42 UTC (permalink / raw)
  To: git

Christian Couder wrote:
> Le samedi 1 mars 2008, Junio C Hamano a écrit :

>> How about allowing multi-valued man.viewer like this:
>>
>>         [man]
>>                 viewer = woman
>>                 viewer = konqueror
>>                 viewer = man
> 
> I tried the above in my .git/config and I get:
> 
> $ git config man.viewer
> woman
> error: More than one value for the key man.viewer: konqueror
> error: More than one value for the key man.viewer: man
> 
> so I guess this can work only in C.

$ git config --get-all man.viewer 

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH AMENDED] help: add "man.viewer" config var to use "woman" or "konqueror"
  2008-03-03  7:42           ` Jakub Narebski
@ 2008-03-03 19:07             ` Christian Couder
  0 siblings, 0 replies; 12+ messages in thread
From: Christian Couder @ 2008-03-03 19:07 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

Le lundi 3 mars 2008, Jakub Narebski a écrit :
> Christian Couder wrote:
> > Le samedi 1 mars 2008, Junio C Hamano a écrit :
> >> How about allowing multi-valued man.viewer like this:
> >>
> >>         [man]
> >>                 viewer = woman
> >>                 viewer = konqueror
> >>                 viewer = man
> >
> > I tried the above in my .git/config and I get:
> >
> > $ git config man.viewer
> > woman
> > error: More than one value for the key man.viewer: konqueror
> > error: More than one value for the key man.viewer: man
> >
> > so I guess this can work only in C.
>
> $ git config --get-all man.viewer

Ooops, you are right, sorry about the noise.

Christian.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2008-03-03 19:02 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-28  4:19 [PATCH 1/2] help: add "man.viewer" config var to use "woman" or "konqueror" Christian Couder
2008-02-29  2:00 ` Xavier Maillard
2008-02-29  7:14   ` Christian Couder
2008-02-29  7:46     ` Junio C Hamano
2008-03-01  1:00       ` Xavier Maillard
2008-03-03  7:20         ` Christian Couder
2008-03-01  1:00     ` [PATCH AMENDED] " Xavier Maillard
2008-03-01  2:14       ` Junio C Hamano
2008-03-03  7:38         ` Christian Couder
2008-03-03  7:42           ` Jakub Narebski
2008-03-03 19:07             ` Christian Couder
2008-02-29  2:00 ` [PATCH 1/2] " Xavier Maillard

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).