Re: Add man parameter for HTML pager
On 2020-06-15 20:21, Ingo Schwarze wrote: > Hi Abel, > > i committed a tweaked version of your patch with these changes: > > - pass pointers to structs around, don't pass structs by value > - no need for an additional automatic variable for the URI > - no need for comments similar to: i++; /* increment i */ > - only use file:// when -O tag was indeed given > That's a point. Thanks. > I dropped changing the filename. I consider that a bug in lynx(1). > Changing behaviour based on the filename "extension" is Microsoft > Windows style and a concept totally alien to Unix. So, i don't > really like the idea to cater to that. Besides, it's trivial to > work around the lynx bug with -force_html without putting weird, > pointless complication into mandoc. > Yes, it is alien but I wanted to try... > Your patch was mangled in at least one way. Most lines were prefixed > with a bogus space character; i did not check whether it was also > mangled in other ways. In this case the mangling didn't hurt because > i had to apply it by hand anyway to check it and to perform the > above tweaks. But when you send patches in the future, please try > to make sure they apply cleanly. Mangled patches can sometimes > cause needless annoyance, and they often delay merging by OpenBSD > developers. > Yes. I used cvs diff way, taken from an user's of a forum... I disabled that boolean format setting as Theo Buehler requested. > I have also documented the new feature in the "HTML Output" subsection > of the mandoc(1) manual page. > Thanks. > Thanks for the suggestion and for the patch! > I is very rare that users who are not BSD or Linux operating system > developers send such substantial mandoc patches. > The latest instance i found in my notes came from Franco Fichtner in 2013. > Thank you by the chance. > Yours, > Ingo > Kindly, Abel.
Re: Add man parameter for HTML pager
It is done. On 2020-06-15 20:31, Theo Buehler wrote: > On Mon, Jun 15, 2020 at 08:21:05PM +0200, Ingo Schwarze wrote: >> Your patch was mangled in at least one way. Most lines were prefixed >> with a bogus space character; i did not check whether it was also >> mangled in other ways. > > User-Agent: Mozilla/5.0 (X11; OpenBSD amd64; rv:68.0) Gecko/20100101 > Thunderbird/68.9.0 > and > Content-Type: text/plain; charset=utf-8; format=flowed > > Thunderbird uses flowed format by default which mangles patches in this > way. Please turn it off before sending the next patch: > > http://kb.mozillazine.org/Plain_text_e-mail_%28Thunderbird%29#Flowed_format >
Re: Add man parameter for HTML pager
On Mon, Jun 15, 2020 at 08:21:05PM +0200, Ingo Schwarze wrote: > Your patch was mangled in at least one way. Most lines were prefixed > with a bogus space character; i did not check whether it was also > mangled in other ways. User-Agent: Mozilla/5.0 (X11; OpenBSD amd64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 and Content-Type: text/plain; charset=utf-8; format=flowed Thunderbird uses flowed format by default which mangles patches in this way. Please turn it off before sending the next patch: http://kb.mozillazine.org/Plain_text_e-mail_%28Thunderbird%29#Flowed_format
Re: Add man parameter for HTML pager
Hi Abel, i committed a tweaked version of your patch with these changes: - pass pointers to structs around, don't pass structs by value - no need for an additional automatic variable for the URI - no need for comments similar to: i++; /* increment i */ - only use file:// when -O tag was indeed given I dropped changing the filename. I consider that a bug in lynx(1). Changing behaviour based on the filename "extension" is Microsoft Windows style and a concept totally alien to Unix. So, i don't really like the idea to cater to that. Besides, it's trivial to work around the lynx bug with -force_html without putting weird, pointless complication into mandoc. Your patch was mangled in at least one way. Most lines were prefixed with a bogus space character; i did not check whether it was also mangled in other ways. In this case the mangling didn't hurt because i had to apply it by hand anyway to check it and to perform the above tweaks. But when you send patches in the future, please try to make sure they apply cleanly. Mangled patches can sometimes cause needless annoyance, and they often delay merging by OpenBSD developers. I have also documented the new feature in the "HTML Output" subsection of the mandoc(1) manual page. Thanks for the suggestion and for the patch! I is very rare that users who are not BSD or Linux operating system developers send such substantial mandoc patches. The latest instance i found in my notes came from Franco Fichtner in 2013. Yours, Ingo
Re: Add man parameter for HTML pager
Hi again, I have added the modifications I think you told me. Also I changed the extension of the tmp file then it can be interpreted as HTML by any browser used as pager. The extension is set when using html format. Now this works: MANPAGER="lynx" ./man -T html -O tag=k pfctl (no need to force_html also) Index: main.c === RCS file: /cvs/src/usr.bin/mandoc/main.c,v retrieving revision 1.252 diff -u -p -r1.252 main.c --- main.c 11 Jun 2020 16:12:14 - 1.252 +++ main.c 15 Jun 2020 10:30:38 - @@ -107,8 +107,8 @@ static void parse(struct mparse *, in static void passthrough(int, int); static void process_onefile(struct mparse *, struct manpage *, int, struct outstate *, struct manconf *); -static void run_pager(struct tag_files *, char *); -static pid_t spawn_pager(struct tag_files *, char *); +static void run_pager(struct outstate, char *); +static pid_t spawn_pager(struct outstate, char *); static void usage(enum argmode) __attribute__((__noreturn__)); static int woptions(char *, enum mandoc_os *, int *); @@ -621,7 +621,7 @@ out: if (outst.tag_files != NULL) { if (term_tag_close() != -1) - run_pager(outst.tag_files, conf.output.tag); + run_pager(outst, conf.output.tag); term_tag_unlink(); } else if (outst.had_output && outst.outtype != OUTT_LINT) mandoc_msg_summary(); @@ -815,7 +815,10 @@ process_onefile(struct mparse *mp, struc if (outst->use_pager) { outst->use_pager = 0; - outst->tag_files = term_tag_init(); + if (outst->outtype == OUTT_HTML) + outst->tag_files = term_tag_init("html"); + else + outst->tag_files = term_tag_init(NULL); } if (outst->had_output && outst->outtype <= OUTT_UTF8) { if (outst->outdata == NULL) @@ -1122,14 +1125,14 @@ woptions(char *arg, enum mandoc_os *os_e * then fork the pager and wait for the user to close it. */ static void -run_pager(struct tag_files *tag_files, char *tag_target) +run_pager(struct outstate outst, char *tag_target) { int signum, status; pid_tman_pgid, tc_pgid; pid_tpager_pid, wait_pid; man_pgid = getpgid(0); - tag_files->tcpgid = man_pgid == getpid() ? getpgid(getppid()) : + outst.tag_files->tcpgid = man_pgid == getpid() ? getpgid(getppid()) : man_pgid; pager_pid = 0; signum = SIGSTOP; @@ -1144,7 +1147,7 @@ run_pager(struct tag_files *tag_files, c if (signum == SIGTTIN) continue; } else - tag_files->tcpgid = tc_pgid; + outst.tag_files->tcpgid = tc_pgid; kill(0, signum); continue; } @@ -1155,7 +1158,7 @@ run_pager(struct tag_files *tag_files, c (void)tcsetpgrp(STDOUT_FILENO, pager_pid); kill(pager_pid, SIGCONT); } else - pager_pid = spawn_pager(tag_files, tag_target); + pager_pid = spawn_pager(outst, tag_target); /* Wait for the pager to stop or exit. */ @@ -1176,19 +1179,20 @@ run_pager(struct tag_files *tag_files, c } static pid_t -spawn_pager(struct tag_files *tag_files, char *tag_target) +spawn_pager(struct outstate outst, char *tag_target) { const struct timespec timeout = { 0, 1 }; /* 0.1s */ #define MAX_PAGER_ARGS 16 char*argv[MAX_PAGER_ARGS]; + char*ofurl; const char *pager; char*cp; size_t cmdlen; int argc, use_ofn; pid_tpager_pid; - assert(tag_files->ofd == -1); - assert(tag_files->tfs == NULL); + assert(outst.tag_files->ofd == -1); + assert(outst.tag_files->tfs == NULL); pager = getenv("MANPAGER"); if (pager == NULL || *pager == '\0') @@ -1218,11 +1222,11 @@ spawn_pager(struct tag_files *tag_files, /* For more(1) and less(1), use the tag file. */ use_ofn = 1; - if (*tag_files->tfn != '\0' && (cmdlen = strlen(argv[0])) >= 4) { + if (*outst.tag_files->tfn != '\0' && (cmdlen = strlen(argv[0])) >= 4) { cp = argv[0] + cmdlen - 4; if (strcmp(cp, "less") == 0 || strcmp(cp, "more") == 0) { argv[argc++] = mandoc_strdup("-T"); - argv[argc++] = tag_files->tfn; + argv[argc++] = outst.tag_files->tfn;
Re: Add man parameter for HTML pager
Thanks for the handy answer, Ingo. I am going to develop it better. On 2020-06-15 04:13, Ingo Schwarze wrote: Hi Abel, Romero Perez, Abel wrote on Mon, Jun 15, 2020 at 03:06:26AM +0200: Romero Perez, Abel wrote: I tried to view the manuals in HTML format with lynx, but I couldn't I assume what you mean is that you could, but the "-O tag" option had no effect. because lynx and links can't accept the /tmp/man.XX... file as an URL... In "-T html" output mode, the tag file isn't even written in the first place. Only the terminal formatter writes a tag file. then I decided to add a new parameter (-U) Absolutely not. Adding new options to man(1) is completely out of the question. Option space is almost full and very messy already: https://mandoc.bsd.lv/man/man.options.1.html It hardly matters that -U is already taken: https://mandoc.bsd.lv/man/man.options.1.html#U You certainly can't take an option that is still free for this purpose, either. to pass the pager a formal file URL scheme as follows: file:///tmp/man.XXX#tag Then man spawns the pager (lynx for my case): lynx -force_html file:///tmp/man.XXX#tag That's an interesting idea. It needs to be done by default when -T html -O tag is given, just like tagging is enabled by default when the pager is less(1). The testing command is: PAGER="" MANPAGER="lynx -force_html" ./man -U -T html -O tag=k pfctl The PAGER="" variable has no effect when MANPAGER is set, see the man(1) manual. So the goal is to make this work: MANPAGER="lynx -force_html" ./man -T html -O tag=k pfctl Sorry, I didn't reviewed the code well. The first diff has some bugs. As follows the stable diff (please, review): I didn't review the diff closely, yet. Obviously, using strncpy(3) or strncat(3) is not permitted, but those should be easy to replace, perhaps with mandoc_asprintf(), not sure yet. It looks like you will have to pass outst into run_pager() and spawn_pager() rather than outst.tag_files, such that you can check for outst.outtype == OUTT_HTML inside spawn_pager(). In any case, thanks for the nice idea! Yours, Ingo
Re: Add man parameter for HTML pager
Hi Abel, Romero Perez, Abel wrote on Mon, Jun 15, 2020 at 03:06:26AM +0200: > Romero Perez, Abel wrote: >> I tried to view the manuals in HTML format with lynx, but I couldn't I assume what you mean is that you could, but the "-O tag" option had no effect. >> because lynx and links can't accept the /tmp/man.XX... file as >> an URL... In "-T html" output mode, the tag file isn't even written in the first place. Only the terminal formatter writes a tag file. >> then I decided to add a new parameter (-U) Absolutely not. Adding new options to man(1) is completely out of the question. Option space is almost full and very messy already: https://mandoc.bsd.lv/man/man.options.1.html It hardly matters that -U is already taken: https://mandoc.bsd.lv/man/man.options.1.html#U You certainly can't take an option that is still free for this purpose, either. >> to pass the pager a formal file URL scheme as follows: >> file:///tmp/man.XXX#tag >> Then man spawns the pager (lynx for my case): >> lynx -force_html file:///tmp/man.XXX#tag That's an interesting idea. It needs to be done by default when -T html -O tag is given, just like tagging is enabled by default when the pager is less(1). >> The testing command is: >> PAGER="" MANPAGER="lynx -force_html" ./man -U -T html -O tag=k pfctl The PAGER="" variable has no effect when MANPAGER is set, see the man(1) manual. So the goal is to make this work: MANPAGER="lynx -force_html" ./man -T html -O tag=k pfctl > Sorry, I didn't reviewed the code well. The first diff has some bugs. > As follows the stable diff (please, review): I didn't review the diff closely, yet. Obviously, using strncpy(3) or strncat(3) is not permitted, but those should be easy to replace, perhaps with mandoc_asprintf(), not sure yet. It looks like you will have to pass outst into run_pager() and spawn_pager() rather than outst.tag_files, such that you can check for outst.outtype == OUTT_HTML inside spawn_pager(). In any case, thanks for the nice idea! Yours, Ingo
Re: Add man parameter for HTML pager
Sorry, I didn't reviewed the code well. The first diff has some bugs. As follows the stable diff (please, review): Index: main.c === RCS file: /cvs/src/usr.bin/mandoc/main.c,v retrieving revision 1.252 diff -u -p -r1.252 main.c --- main.c 11 Jun 2020 16:12:14 - 1.252 +++ main.c 15 Jun 2020 01:05:12 - @@ -107,8 +107,8 @@ static void parse(struct mparse *, in static void passthrough(int, int); static void process_onefile(struct mparse *, struct manpage *, int, struct outstate *, struct manconf *); -static void run_pager(struct tag_files *, char *); -static pid_t spawn_pager(struct tag_files *, char *); +static void run_pager(struct tag_files *, char *, int use_furl); +static pid_t spawn_pager(struct tag_files *, char *, int use_furl); static void usage(enum argmode) __attribute__((__noreturn__)); static int woptions(char *, enum mandoc_os *, int *); @@ -140,6 +140,7 @@ main(int argc, char *argv[]) size_t i, ib, ssz; int options; /* Parser options. */ int show_usage;/* Invalid argument: give up. */ + int use_furl; /* Use file:///tmp/man. URL scheme */ int prio, best_prio; int startdir; int c; @@ -183,6 +184,7 @@ main(int argc, char *argv[]) options = MPARSE_SO | MPARSE_UTF8 | MPARSE_LATIN1; os_e = MANDOC_OS_OTHER; os_s = NULL; + use_furl = 0; /* Formatter options. */ @@ -195,7 +197,7 @@ main(int argc, char *argv[]) outmode = OUTMODE_DEF; while ((c = getopt(argc, argv, - "aC:cfhI:iK:klM:m:O:S:s:T:VW:w")) != -1) { + "aC:cfhI:iK:klM:m:O:S:s:T:VW:wU")) != -1) { if (c == 'i' && search.argmode == ARG_EXPR) { optind--; break; @@ -301,6 +303,9 @@ main(int argc, char *argv[]) case 'w': outmode = OUTMODE_FLN; break; + case 'U': + use_furl = 1; + break; default: show_usage = 1; break; @@ -621,7 +626,7 @@ out: if (outst.tag_files != NULL) { if (term_tag_close() != -1) - run_pager(outst.tag_files, conf.output.tag); + run_pager(outst.tag_files, conf.output.tag, use_furl); term_tag_unlink(); } else if (outst.had_output && outst.outtype != OUTT_LINT) mandoc_msg_summary(); @@ -639,7 +644,7 @@ usage(enum argmode argmode) "\t [-T output] [-W level] [file ...]\n", stderr); break; case ARG_NAME: - fputs("usage: man [-acfhklw] [-C file] [-M path] " + fputs("usage: man [-acfhklwU] [-C file] [-M path] " "[-m path] [-S subsection]\n" "\t [[-s] section] name ...\n", stderr); break; @@ -1122,7 +1127,7 @@ woptions(char *arg, enum mandoc_os *os_e * then fork the pager and wait for the user to close it. */ static void -run_pager(struct tag_files *tag_files, char *tag_target) +run_pager(struct tag_files *tag_files, char *tag_target, int use_furl) { int signum, status; pid_tman_pgid, tc_pgid; @@ -1155,7 +1160,7 @@ run_pager(struct tag_files *tag_files, c (void)tcsetpgrp(STDOUT_FILENO, pager_pid); kill(pager_pid, SIGCONT); } else - pager_pid = spawn_pager(tag_files, tag_target); + pager_pid = spawn_pager(tag_files, tag_target, use_furl); /* Wait for the pager to stop or exit. */ @@ -1176,11 +1181,14 @@ run_pager(struct tag_files *tag_files, c } static pid_t -spawn_pager(struct tag_files *tag_files, char *tag_target) +spawn_pager(struct tag_files *tag_files, char *tag_target, int use_furl) { const struct timespec timeout = { 0, 1 }; /* 0.1s */ #define MAX_PAGER_ARGS 16 +#define MAX_FILE_TAG 30 +#define MAX_FILE_URL 33 /* 33 is file:///tmp/man.X...#\0 */ char*argv[MAX_PAGER_ARGS]; + charofurl[MAX_FILE_URL + MAX_FILE_TAG]; const char *pager; char*cp; size_t cmdlen; @@ -1230,8 +1238,22 @@ spawn_pager(struct tag_files *tag_files, } } } - if (use_ofn) - argv[argc++] = tag_files->ofn; + + if (use_ofn) { + if (use_furl) { + strncpy(ofurl, "file://", MAX_FILE_URL); + strncat(ofurl, ta