Re: mdocml: Renamed mandoc.c to libmandoc.c.

Re: mdocml: Renamed mandoc.c to libmandoc.c.

[Ingo Schwarze]

Hi Kristaps,

kristaps@mdocml.bsd.lv wrote on Mon, Jul 05, 2010 at 04:00:56PM -0400:

> Log Message:
> -----------
> Renamed mandoc.c to libmandoc.c.  This is in the efforts of getting a
> cleaner namespace for functions across the entire system (mandoc.h:
> getting parsed-string values, or declarations necessary for the AST
> data), and compiler functions (libmandoc.h: back-end functions and
> declarations).

I dislike this change.
I think the attempt to get a cleaner namespace this way is futile.

The whole concept of x.h vs. libx.h doesn't work out, there is
no real separation.  As far as i understand, the idea is to have
the pure AST parts in x.h, such that x.h is self-contained
except for requiring mandoc.h, while libx.h is supposed to be
the interface to one specific X parser which happens to be
using x.h, while x.h does not depend on the specific parser
interface defined by libx.h.

But this is already busted, because lots of functions from man.h
use the forward declaration of struct man from libman.h,
and lots of functions from mdoc.h use the forward declaration
of struct mdoc from libmdoc.h, so the AST cannot exist without
the one specific parser we have.  I don't say that is a problem,
because i guess nobody will ever try to write an alternative parser
using man.h or mdoc.h.  My point is just that the separation
of x.h and libx.h is mere fiction.  We could as well put
x.h and libx.h into one header (of course, don't, that would
just cause useless churn).

That said, i see no point in extending this failed concept to
mandoc.h and libmandoc.h.  I didn't speak up yet because i don't
feel that strongly about which function is defined in which header
and implemented in which .c-file, so i was mostly happy with
what we have.

But i do care about CVS history, so i think renaming mandoc.c
is a really bad idea.  We lose history for no reason.

I think this change ought to be undone.  That is, undone on the
RCS level, not just reverted by renaming the file once more.

Yours,
  Ingo


P.S.
Btw., while trying to figure out how headers and .c-files are
related, i found a few weirdnesses:

 * libmdoc.h declares mdoc_isescape and mdoc_atotime
   which are neither defined nor used
 * man.c: man_pmacro lacks static on its definition
 * man.c implements man.h without explicitely including it;
   it is only included via libman.h
 * man_macro.c: blk_close, blk_exp, blk_exp, in_line_eoln
   lack static on their definitions
 * mdoc.c: mdoc_node_free and mdoc_pmacro
   lack static on their definitions
 * mdoc.c implements mdoc.h without explicitely including it
   it is only included via libmdoc.h

These are probably easily fixed, while the following are not:

 * roff.h forward-declares struct roff
   which is not declared in any header at all,
   but only in roff.c, using more local roff.c declarations.
   That is, the roff.h interface is *not* independent of the
   particular roff.c implementation.
 * The forward declaration of struct man and struct mdoc
   in main.h is ugly, too, because effectively, that makes
   main.h depend on libman.h and libmdoc.h, thus also on
   man.h, mdoc.h and mandoc.h.  Now, main.h is used all over
   the place (html.c, man_html.c, man_term.c, mdoc_html.c,
   mdoc_term.c, term.c, term_ascii.c, term_ps.c, tree.c),
   which means that all these files depend on the full AST
   structure and BOTH parsers - yes, even the man frontends
   are not cleanly separated from the mdoc parser and vice
   versa.
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

Re: mdocml: Renamed mandoc.c to libmandoc.c.

[Kristaps Dzonsons]

Ingo,

> I dislike this change.
> I think the attempt to get a cleaner namespace this way is futile.

Yep, I agree, but was too tired to revert it last night.  Basically, I 
was trying to put together another roff_getstr() interface.  I was 
starting to abstract everything then realised---as you wrote in your 
message and I failed to see at the time---that of course you /can't/ 
have roff_getstr() in term.c because the value would be out-of-band. 
The fix is pretty mechanical (namely, doing it in mdoc_validate.c where 
the string pointer is reallocated on-the-fly if the escape is flagged), 
but I didn't have the energy to do it.

> I think this change ought to be undone.  That is, undone on the
> RCS level, not just reverted by renaming the file once more.

Sigh.  Yep.

>  * libmdoc.h declares mdoc_isescape and mdoc_atotime
>    which are neither defined nor used

I'll remove these (they've long since gone into libmandoc.h).

>  * man.c: man_pmacro lacks static on its definition

Will fix.

>  * man.c implements man.h without explicitely including it;
>    it is only included via libman.h

So do all man_* and mdoc_* files (with libman.h and libmdoc.h, resp.). 
If this bothers you, it's a mechanical change.

>  * man_macro.c: blk_close, blk_exp, blk_exp, in_line_eoln
>    lack static on their definitions
>  * mdoc.c: mdoc_node_free and mdoc_pmacro
>    lack static on their definitions

Will fix.

>  * mdoc.c implements mdoc.h without explicitely including it
>    it is only included via libmdoc.h

Ah--see above.

> 
> These are probably easily fixed, while the following are not:
> 
>  * roff.h forward-declares struct roff
>    which is not declared in any header at all,
>    but only in roff.c, using more local roff.c declarations.
>    That is, the roff.h interface is *not* independent of the
>    particular roff.c implementation.

I'm aware of this.  The fix, of course, is pretty easy: dumping the 
structures into an extern.h file and keeping the compiler functions 
within mdoc.h.

>  * The forward declaration of struct man and struct mdoc
>    in main.h is ugly, too, because effectively, that makes
>    main.h depend on libman.h and libmdoc.h, thus also on
>    man.h, mdoc.h and mandoc.h.  Now, main.h is used all over
>    the place (html.c, man_html.c, man_term.c, mdoc_html.c,
>    mdoc_term.c, term.c, term_ascii.c, term_ps.c, tree.c),
>    which means that all these files depend on the full AST
>    structure and BOTH parsers - yes, even the man frontends
>    are not cleanly separated from the mdoc parser and vice
>    versa.

Ja.  I don't worry about this too much, as fixing it's (1) not necessary 
right now, and (2) easy to do.

For the record, there should ideally be a division as follows:

               +-----libmandoc----+
            libmdoc   libman   libroff
               |         |        |
               +---------+--------+
                         |
                        main
                         |
             +-----------+-----------+
             |           |           |
           term---------html--------tree
         ps/ascii    html/xhtml  pine/birch

It's clear that /all/ of these need the mdoc.h and man.h structures. 
Only the libmdoc/man/roff/main need the parse/access functions.

Ingo, your thoughts: after the `Bk' stuff is checked in, I make another 
little tag, we clean up, fix this architectural cruft while it's fresh 
in our minds, then tag and make a real release?

Kristaps
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

Re: mdocml: Renamed mandoc.c to libmandoc.c.

[Kristaps Dzonsons]

>> I think this change ought to be undone.  That is, undone on the
>> RCS level, not just reverted by renaming the file once more.

I don't understand this.  Isn't a `cvs add' good enough to resurrect the
file after it's been removed?
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

Re: mdocml: Renamed mandoc.c to libmandoc.c.

[Ingo Schwarze]

Hi Kristaps,

>> I think this change ought to be undone.  That is, undone on the
>> RCS level, not just reverted by renaming the file once more.

> I don't understand this.  Isn't a `cvs add' good enough to resurrect the
> file after it's been removed?

Yes, it is, i only worried about the history, but even that it fully
preserved, as i just tested:

 * create a file (rev. 1.1)
 * change a line (rev. 1.2)
 * delete the file (rev. 1.3)
 * resurrect the file (rev. 1.4)

I assumed cvs annotate would then show 1.4 for all lines.
But it doesn't, it still shows 1.1 and 1.2 just like it did
before the removal; cvs annotate apparently just ignores the
deleted revision.

Thus, you are right, just cvs add & commit is the way to go.

Yours,
  Ingo
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

Re: mdocml: Renamed mandoc.c to libmandoc.c.

[Ingo Schwarze]

Hi Kristaps,

Kristaps Dzonsons wrote on Tue, Jul 06, 2010 at 11:24:18AM +0200:

[...]
> The fix is pretty mechanical (namely, doing it in mdoc_validate.c
> where the string pointer is reallocated on-the-fly if the escape is
> flagged), but I didn't have the energy to do it.

Hm, i shall wait and see what you are driving at, though i'm
a bit suspicious whether the parser backend it the best place
for this.

Apparently, groff supports constructs like the following,
see /usr/src/gnu/usr.bin/groff/tmac/tmac.doc:

.nr aP 3
.ds A3 Fl
.\*(A\n(aP

resolving via ".\*(A3" to ".Fl" which will be evaluated as -
the mdoc(7) flag macro.  Obfuscation, anybody?

I don't say we need to implement all that, probably most such
stuff will be rarely needed, but when we have the choice, it
is perhaps worthwhile doing some of the expansions on the
preprocessor level, maybe even iteratively.

Of course, things like special characters and font escapes,
which depend critically on the choice of the frontend, have
no business in the preprocessor, but belong into the frontend.
Yet, the escape sequence parsing logic for \*x, \*(xx, \*[xxx]
is the same for both cases.  Not sure how to organize the code
to deal with that situation, to minimize code duplication as
well as duplicate processing...

In any case, the one place where i would least expect to find
escape sequence parsing is in the mdoc/man parser backends.

[...]
>> * man.c implements man.h without explicitely including it;
>>   it is only included via libman.h

> So do all man_* and mdoc_* files (with libman.h and libmdoc.h,
> resp.). If this bothers you, it's a mechanical change.

I tend to directly include a header when i directly use symbols
defined there, even when another header i'm using implicitely
includes it anyway - but that is a matter of style and not so
important.

What i do find surprising is that a .c-file defining a function
(say, man.c defining man_parseln) does not explicitely include
the header declaring it (i.e., man.h declaring man_parseln),
but relies on another header it happens to be using (libman.h)
to implicitely include man.h.

As far as i see, that happens in two cases only:
  man.c   not including  man.h   directly
  mdoc.c  not including  mdoc.h  directly

But even that is certainly not critical.


[...]
> Ja.  I don't worry about this too much, as fixing it's
> (1) not necessary right now,

Yes, i agree there is no need to reshuffle the headers
right now.  I only looked at it because i was under the
impression you were about to start that kind of reshuffling,
and wanted to figure out how good or bad the situation
actually is, and whether one could expect perfect results
with reasonable effort.

Right now, let's not do more than simple fixes, let's not
move files around or change the basic structure, it works
well enough.


> For the record, there should ideally be a division as follows:
> 
>               +-----libmandoc----+
>            libmdoc   libman   libroff
>               |         |        |
>               +---------+--------+
>                         |
>                        main
>                         |
>             +-----------+-----------+
>             |           |           |
>           term---------html--------tree
>         ps/ascii    html/xhtml  pine/birch
> 
> It's clear that /all/ of these need the mdoc.h and man.h structures.
> Only the libmdoc/man/roff/main need the parse/access functions.

Yes, that basic picture is fine.  Except that many functions in man.h
and mdoc.h use pointers to struct man and struct mdoc, which contain
all the parser functions, so there is a bit of a looping.  But again,
no need to unwind that right now.

> Ingo, your thoughts: after the `Bk' stuff is checked in, I make
> another little tag, we clean up, fix this architectural cruft while
> it's fresh in our minds, then tag and make a real release?

Sounds good.  So i will do the main merging to OpenBSD after the
little tag, and perhaps we can then also put in two small fixes
to .Sm and .No i'm currently preparing.

Yours,
  Ingo
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

Re: mdocml: Renamed mandoc.c to libmandoc.c.

[Kristaps Dzonsons]

Ingo,

> resolving via ".\*(A3" to ".Fl" which will be evaluated as -
> the mdoc(7) flag macro.  Obfuscation, anybody?

That's the nail in the coffin to put this in libroff, as once the 
character is re-written, it can be pumped (ROFF_RERUN) back in for more 
processing.  Sigh.

> I don't say we need to implement all that, probably most such
> stuff will be rarely needed, but when we have the choice, it
> is perhaps worthwhile doing some of the expansions on the
> preprocessor level, maybe even iteratively.
> 
> Of course, things like special characters and font escapes,
> which depend critically on the choice of the frontend, have
> no business in the preprocessor, but belong into the frontend.
> Yet, the escape sequence parsing logic for \*x, \*(xx, \*[xxx]
> is the same for both cases.  Not sure how to organize the code
> to deal with that situation, to minimize code duplication as
> well as duplicate processing...

mandoc_special() should be merged with a2roffdeco(), which would address 
this.

> In any case, the one place where i would least expect to find
> escape sequence parsing is in the mdoc/man parser backends.

Warnings about malformed escape sequences should be raised somewhere, 
and this shouldn't be in the front-end.

Thanks,

Kristaps
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

Re: mdocml: Renamed mandoc.c to libmandoc.c.

[Ingo Schwarze]

Hi Kristaps,

Kristaps Dzonsons wrote on Wed, Jul 07, 2010 at 12:56:37PM +0200:

>> resolving via ".\*(A3" to ".Fl" which will be evaluated as -
>> the mdoc(7) flag macro.  Obfuscation, anybody?

> That's the nail in the coffin to put this in libroff, as once the
> character is re-written, it can be pumped (ROFF_RERUN) back in for
> more processing.  Sigh.

From inspection, your code looks good!

It is still handling predefined strings later than user-defined
strings, but that is not so important that it needs to be fixed now.
In both old and new groff, the following works (horrors!-):

  .ds pi surprising
  This is \*(\*(Pi.

Which gives us:

  This is surprising.

The predefined string \*(Pi evaluates to "pi",
and we have defined \*(pi ourselves.  Oh well.  :-)

Yours,
  Ingo
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv