When user clicks on broken link, console reports

usage: man [-] [adFlrt] [-M path] etc........

implying man command not being given correct arguments.

This is probably another linux/solaris format difference. Moving to
urgent to force wipro to inspect the problem.

I forgot; this should be under yelp, not help-browser.

hmm .. I would guess this is really a problem in libgnome, that
gnome2-man2html creates broken links.

I will have to reinvestigate this, will get back to if it's a yelp or
a libgnome-bug.

Created attachment 7561 [details] [review]
proposed patch

adding andersca since he is libgnome-maintainer. (and I suck at
autoconf-stuff :)

moved to libgnome

Reassigning to andersca as well.

Oh, and Keelin, are you still seeing this? It seems to work here on Linux.

not that it really has anything to do with this patch, I'm thinking of
moving in the gnome2-man2html into Yelp and make it an internal
function of Yelp for now. This gives me better control of the
error-messages and such and can take better steps in informing the
user about them.

The proposed patch should never go in.

* Please write a proper autoconf check for what parameters 'man'
accepts. Never ever invoke the 'host' command inside 'configure.in'.
If there is a POSIX standard for what you're looking for, start with that.

* Don't violate C {} nesting with an #ifdef.

Moving down to high; would still be good to get patched and applied,
though, if it is done correctly.

I agree with raja here, but it should be fixed.

ok, so does anyone have another fix for this or can inform me on how
to do it?

Raja, could you suply a better solution?

*** Bug 80217 has been marked as a duplicate of this bug. ***

Fairly major and we have a patch that would work, even if ugly, so
leaving 2.0.0 but please feel free to punt to 2.0.1 if you don't think
this will/should happen

The 'configure.in' part of the code is:

  AC_CANONICAL_HOST
  case $host in
  *-linux-gnu | *-irix6*) AC_DEFINE(HAVE_MAN_WHEREIS, [man(1)
understands -w option]);;
  *-solaris*) AC_DEFINE(HAVE_MAN_DEBUG, [man(1) has a -d debug option]);;
  esac

There's no need to patch Makefile.am.

The C part of the code is: somewhere add this code

  #if defined HAVE_MAN_WHEREIS
  #define MAN_CAN_FIND_SOURCE
  const char *man_source_locator = "man -w %s";
  const char *man_source_locator_with_section = "man -w %c %s";
  #elif defined HAVE_MAN_DEBUG
  #define MAN_CAN_FIND_SOURCE
  const char *man_source_locator = "man -d %s | sed -n 's,^[
\t]*unformated[ \t]*=[ \t]*,,p'";
  const char *man_source_locator_with_section = "man -d -s %c %s | ...";
  #endif

and below, replace 

  snprintf(cmdline, sizeof(cmdline), "man -w %c %s", section, infile);

with

  #ifdef MAN_CAN_FIND_SOURCE
  snprintf(cmdline, sizeof(cmdline), man_source_locator_with_section,
section, infile);
  #else
  // do whatever necessary to indicate that the function failed
  #endif

Hopefully the folks at Wipro can put together a patch from this, test
it, etc.  I can't do it right now since I haven't yet set up my source
tree.

Again, irrespective of urgency, no way should ugly, unmaintainable
code go in.

(BTW, to the Wipro guys who may be interested, in the 'sun-patches'
directory, 

  grep -l uname */*.diff

gives me

  gnome-utils/073-70827-w.diff
  libgnome/074-76331-w.diff
  nautilus/122-00000-w.diff

They should be treated similarly.)

Sorry about the wordwrap.

  *-linux-gnu | *-irix6*) 
     AC_DEFINE(HAVE_MAN_WHEREIS, [man(1) understands -w option]);;

...

  #elif defined HAVE_MAN_DEBUG
  #define MAN_CAN_FIND_SOURCE
  const char *man_source_locator = "man -d %s " 
             "| sed -n 's,^[ \t]*unformated[ \t]*=[ \t]*,,p'";
  const char *man_source_locator_with_section = "man -d -s %c %s "
             "| ...";
  #endif

(though it may be obvious what I meant.)

Sorry again.  I'm typing from memory.  So, you'll have to forgive
small mistakes in the suggestions.

The syntax for AC_DEFINE is something like

     AC_DEFINE(HAVE_MAN_WHEREIS, 1, [man(1) understands -w option]);;

Created attachment 8247 [details] [review]
proposed patch

I think you missed my last message.

 --- libgnome/acconfig.h	Thu Nov  1 00:29:31 2001
 +++ libgnome_new/acconfig.h	Tue May  7 16:22:24 2002
 +#undef HAVE_MAN_WHEREIS 
 +#undef HAVE_MAN_DEBUG
 +#undef MAN_CAN_FIND_SOURCE

Please don't touch acconfig.h.

  --- libgnome/configure.in	Mon May  6 00:08:00 2002  
  +++ libgnome_new/configure.in	Tue May  7 16:20:52 2002
  @@ -44,6 +44,12 @@ AC_SUBST(LIBGNOME_CURRENT)

+AC_CANONICAL_HOST
+   case $host in
+   *-linux-gnu | *-irix6*) AC_DEFINE(HAVE_MAN_WHEREIS, [man(1) 
understands -w option]);;
+   *-solaris*) AC_DEFINE(HAVE_MAN_DEBUG, [man(1) has a -d debug
option]);;
+   esac

Change these to

  AC_DEFINE(HAVE_MAN_WHEREIS, 1, [man(1) understands -w option]);;

and

  AC_DEFINE(HAVE_MAN_DEBUG, 1, [man(1) has a -d debug option]);;

(note the '1')

Also, in

+#ifdef MAN_CAN_FIND_SOURCE
+
	    g_snprintf(cmdline, sizeof(cmdline),
man_source_locator_with_section, section, infile);
+#else
+
g_message("Path for man page not found"); 
+#endif

The alternative has bad indentation.  The code there appears to be
incomplete.  I'm sure you want to do more than just show a message. 
You don't want the 'popen' further down to run on an uninitialized
'cmdline'.  You probably want this code (cut-and-pasted from further
down in the same file):

          {
            printf("<HTML><HEAD><TITLE>Document not found</TITLE>\n"
                   "</HEAD><BODY>"
                   "I don't know how to find the document \"%s\" "
                   "on your system.\n"
                   "</BODY></HTML>\n", infile);

                return 4;
          }

Created attachment 8278 [details] [review]
proposed patch

Please fix indentation.  Otherwise looks OK.

I guess it can be committed once all variants are tested (temporarily
unset MAN_CAN_FIND_SOURCE and check to see if it compiles and runs).

Anders?

I've committed this to CVS now.