[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Bug#38107: new patch for man-db japanese support (need help for other langs)



Hi.

ukai> Hmm, As I checked your patch, it does not work correctly.  
ukai> The following for() loop never run, because at first j = 0, then
ukai> j && lang_table[j].lang will be false. so it would never check lang_table.

I also noticed using your (Fabrizio's) patch, man does segmatation fault
when it is given a non-existing man page. You can check it by "man zzz".

This is caused by "if ( ! *lang ) {" at line 1327. lang has type of (char *),
so this should be "if ( ! lang ) {", I think.

ukai> I'm not sure what you intend here exactly, but I think it should be
ukai> the followings, shouldn't it?
ukai> 
ukai> 				int j;
ukai> 				for ( j=0; lang_table[j].lang; j++ ) {
ukai> 					if (( strncmp( lang_table[j].lang, lang
ukai> 						, strlen( lang_table[j].lang)) == 0 )
ukai> 					||  ( lang_table[j].lang[0] == '*' )) {
ukai> 						roff_device = lang_table[j].device;
ukai> 						troff = 1;
ukai> 						putenv( strappend ( 0
ukai> 							,"LESSCHARSET="
ukai> 							, lang_table[j].charset
ukai> 							, 0));
ukai> 						break;
ukai> 					}
ukai> 				}
ukai> 
ukai> With this fix, it works fine for LANG=ja. Thanks!

When I use man-db without the change above (! *lang -> ! lang),
manpages are displayed on "LANG=C" with pager. But on "LANG=ja_JP.ujis",
manpages are displayed without pager.

Here are the output from ps axwf:

(for "LANG=C man ls" :)
 3802  p2 T    0:00  \_ man ls 
 3805  p2 T    0:00  |   \_ sh -c /bin/gzip -dc '/var/cache/man/fsstnd/cat1/ls.1.gz' | { export MAN_PN LESS; MAN_PN='ls(1)'; LESS="$LESS\$-Pm\:\$ix8mPm Manual p
 3806  p2 T    0:00  |       \_ /bin/gzip -dc /var/cache/man/fsstnd/cat1/ls.1.gz 
 3807  p2 T    0:00  |       \_ sh -c /bin/gzip -dc '/var/cache/man/fsstnd/cat1/ls.1.gz' | { export MAN_PN LESS; MAN_PN='ls(1)'; LESS="$LESS\$-Pm\:\$ix8mPm Manu
 3808  p2 T    0:00  |           \_ /usr/bin/jless -i 
 3816  p2 R    0:00  \_ ps xwf 

(for "LANG=ja_JP.ujis man ls" :)
  777  p2 S    0:01 -bash 
 3787  p2 T    0:00  \_ man ls 
 3790  p2 T    0:00  |   \_ sh -c /usr/bin/zsoelim '/tmp/zman03787aaa' | /usr/bin/groff -mandoc -Tnippon 
 3791  p2 T    0:00  |       \_ /usr/bin/zsoelim /tmp/zman03787aaa 
 3792  p2 T    0:00  |       \_ /usr/bin/groff -mandoc -Tnippon 
 3793  p2 T    0:00  |           \_ troff -msafer -mandoc -Tnippon 
 3794  p2 T    0:00  |           \_ grotty 
 3800  p2 R    0:00  \_ ps xwf 
 3801  p2 R    0:00  \_ -bash 

and using the change of "[! *lang] -> [! lang]", the manpages are
displayed without pager in both case (either "LANG=C" or "LANG=ja_JP.ujis")

I find that the line "if (troff) {" at 1790 prevent to use pager 
when troff is true. And the comment begining at line 1046 say

                        case 'T':
                                /* @@@ traditional nroff knows -T,
                                   troff does not (gets ignored).
                                   All incarnations of groff know it.
                                   Why does -T imply -t? */

I wish to know "Why does -T imply -t?" Is it required ?
I am not satisfied if manpages are always shown without pager.

By the way, following this comment, there are lines below.

                                /* as this is an optional argument */
                                roff_device = (optarg ? optarg : "ps");
                                troff = 1;
                                break;

But there is a line at 478:

static const char args[] = "7DlM:P:S:adfhkVum:p:we:L:cr:";

and a line at 466:

static const char args[] = "7DlM:P:S:adfhkVum:p:tT:we:L:Zcr:X";

Both of these two lines have "T:" as a optstring for getopt_long (at 983),
but manpage of getopt_long say:

       optstring  is  a  string  containing the legitimate option
       characters.  If such a character is followed by  a  colon,
       the  option  requires  an  argument,  so  getopt  places a
       pointer to the following text in the same argv-element, or
       the  text  of  the following argv-element, in optarg.  Two
       colons mean an option takes an optional arg; if  there  is
       text  in  the  current  argv-element,  it  is  returned in
       optarg, otherwise optarg is set to zero.  This  is  a  GNU
       extension.

so if "-T" option takes an optional arg, then args[] should have
"T::" as a part of it. (at least, any GNU system)

In fact, you can check it by "man -T ls". 
This output the error message "What manual page do you want?".

Again, we can use "-t" explicitly when we needs it, I don't think
it is necessary to imply "-t" by "-T", and let "-t" be default is
not comfortable for me at all.

I attach the patch against original 2.3.10-69i and against 
the previous patch from Fabrizio.

Your implementation is cool approach, Fabrizio. But manpages are
important for many novice users and even trifle things such that 
man-db does not use pager can easily be obstacle for us.

Please change the behavior of man-db to use pager as default.
Thanks.

-- 
  Taketoshi Sano: <sano@debian.org>,<sano@debian.or.jp>,<kgh12351@nifty.ne.jp>
--- man.c.orig	Wed Sep 15 17:13:04 1999
+++ man-db-2.3.10/src/man.c	Wed Sep 15 17:30:13 1999
@@ -142,6 +142,18 @@
 #  define STDERR_FILENO 2
 #endif
 
+char * lang;
+struct {
+	char *	lang;
+	char *	device;
+	char *	charset;
+} lang_table[] =	{
+	/* LANG		roff_device	LESSCHARSET */
+	{ "ja"		, "nippon"	, "ja"		},
+	{ "cs"		, "latin2"	, "latin2"	},
+	{ "*"		, "latin1"	, "latin1"	},
+	{ 0		, 0		, 0		} };
+
 /* external formatter programs, one for use without -t, and one with -t */
 #define NFMT_PROG "./mandb_nfmt"
 #define TFMT_PROG "./mandb_tfmt"
@@ -317,6 +329,37 @@
 }
 #endif /* MAN_CATS */
 
+char * lang_dir( char * filename)
+{
+	char *ld;	/* the lang dir: point to static data */
+	char *fm;	/* the first "/man/" dir */
+	char *sm;	/* the second "/man?/" dir */
+
+	ld = "";
+	if ( ! filename ) 
+		return ld;
+
+	if ( ! (fm = strstr( filename, "/man/")) )
+		return ld;
+	if ( ! (sm = strstr( 3+fm, "/man")) )
+		return ld;
+	if ( sm == 4+fm )
+		return ld;
+	if ( sm[5] != '/' )
+		return ld;
+	if ( ! strchr( "123456789lno", sm[4]) )
+		return ld;
+	/* found a lang dir */
+	fm += 5;
+	if ( ! (sm = strchr( fm, '/')) )
+		return ld;
+	ld = xstrdup ( fm);
+	ld[sm-fm] = '\0';
+	if (debug)
+		fprintf (stderr, "found lang dir element %s\n", ld);
+	return ld;
+}
+
 static __inline__ void gripe_system (char *command, int status)
 {
 	error (CHILD_FAIL, 0, _( "command exited with status %d: %s"), status, command);
@@ -841,8 +884,6 @@
 	if (optind == argc)
 		gripe_no_name (NULL);
 
-	putenv("LESSCHARSET=latin1");
-
 	signal( SIGINT, int_handler);
 
 	/* man issued with `-l' option */
@@ -1281,6 +1322,26 @@
 		char *dev;	/* either " -T<mumble>" or "" */
 		int using_tbl = 0;
 
+		/* load the roff_device value dependent on the language dir in path */
+		if ( ! roff_device ) {
+			if ( ! lang ) {
+				roff_device = "latin1";
+			} else {
+				int j;
+				for ( j=0; lang_table[j].lang; j++ ) { 
+					if (( strncmp( lang_table[j].lang, lang
+						, strlen( lang_table[j].lang)) == 0 )
+					||  ( lang_table[j].lang[0] == '*' )) {
+						roff_device = lang_table[j].device;
+						putenv( strappend ( 0
+							,"LESSCHARSET="
+							, lang_table[j].charset
+							, 0));
+						break;
+					}
+				}
+			}
+		}
 		/* tell grops to guess the page size */
 		if ( roff_device && strcmp( roff_device, "ps") == 0 )
 			roff_device = strappend( NULL, "ps -P-g ", NULL);
@@ -2024,6 +2085,7 @@
 
 			if (debug)
 				fprintf (stderr, "found ultimate source file %s\n", man_file);
+			lang = lang_dir (man_file);
 
 			cat_file = find_cat_file (path, man_file, sec);
 			found += display (path, man_file, cat_file, title);
@@ -2131,6 +2193,7 @@
 
 			if (debug)
 				fprintf (stderr, "found ultimate source file %s\n", man_file);
+			lang = lang_dir (man_file);
 
 			cat_file = find_cat_file (path, man_file, in->ext);
 			found += display (path, man_file, cat_file, title);
--- man.c.fab	Wed Sep 15 17:13:06 1999
+++ man-db-2.3.10/src/man.c	Wed Sep 15 17:30:13 1999
@@ -1324,22 +1324,21 @@
 
 		/* load the roff_device value dependent on the language dir in path */
 		if ( ! roff_device ) {
-			if ( ! *lang ) {
+			if ( ! lang ) {
 				roff_device = "latin1";
 			} else {
 				int j;
-				for ( j=0; j && lang_table[j].lang; j++ ) {
+				for ( j=0; lang_table[j].lang; j++ ) { 
 					if (( strncmp( lang_table[j].lang, lang
 						, strlen( lang_table[j].lang)) == 0 )
 					||  ( lang_table[j].lang[0] == '*' )) {
 						roff_device = lang_table[j].device;
-						troff = 1;
 						putenv( strappend ( 0
 							,"LESSCHARSET="
 							, lang_table[j].charset
 							, 0));
+						break;
 					}
-					j= -1;
 				}
 			}
 		}

Reply to: