[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: lynx-dev [PATCH] misc [VH] changes
|
From: |
Vlad Harchev |
|
Subject: |
Re: lynx-dev [PATCH] misc [VH] changes |
|
Date: |
Sun, 31 Oct 1999 13:06:01 +0400 (SAMT) |
On Sat, 30 Oct 1999, Klaus Weide wrote:
> [ I have not tried the patches yet, just some comments ]
>
> On Fri, 29 Oct 1999, Vlad Harchev wrote:
>
> > * Old psrcview bug fixed - markup was unbalanced in some situations
>
> Does it solve the following problem?
>
> <HTML><TITLE>test</TITLE>
> some text
> <UNKNOWN IRRELEVANT=something>
> some text
> <UNKNOWN IRRELEVANT=something>
> some text
> <UNKNOWN IRRELEVANT=something>
> some text
> </HTML>
No, it solves the problems with following piece of html
<table border border> some text
(Ie known tag, and more than 1 attributes without value).
> Note how the first UNKNOWN tag with an attribute screws up the
> rendering of text and tags that follow. This probably only appears
> *without* color style (please check).
Yes, this only appears without color style. There is a better solution -
enclose only tagname in the tags specified by HTMLSRC_TAG or HTMLSRC_BADTAG -
so that psrc rules like '!b:b' (that were needed since entire text
between '<' and '>' was enclosed in tags) won't be required at all. I will
send a patch soon.
> It goes away when I change some of the HTMLSRC_* rules:
> HTMLSRC_ATTRVAL::
> HTMLSRC_HREF:!b:b
>
> If that problem remains, the defaults should change (to the above
> unless you have something better) so that at least it doesn't appear
> by default.
I just wonder that this problem is reported only now. Either nobody use
psrcview, or don't encounter bad html.
> > * Content of SAMP tag made justificable
> (I don't think that's a word. :)) ^
> > * Implemented HTStyleChange pooling - sizeof(HTStyleChange) is 4 times
> > smaller
> > than before, and only necessary number of stylechanges is allocated
> > now,
> > instead of some constant (1024 by default).
>
> Comments in your code suggest this could be used for other things.
> If so that's great, code reuse is a good thing... I couldn't see
> easily how and for what, though. Maybe you could add some more
> comments. Specifically, what is a "pool" (in the sense of your
> code)? Someone who doesn't know that (like me) may reinvent the
> wheel later, which could be avoided.
OK, I'll add comments in the next patch (but seems the comment
/*these are generic macors for any pools (provided those structures have the
same members as HTStyleChangePool) - VH */
tells clearly that the pool is a structure that has the members with the same
names as in _HTStyleChangePool).
I was thinking on creating small LYPool.h out of those macros - to aid code
reuse. May be this will be better?
> > * Functionality of dont-wrap-pre improved and extened. Now it's possible to
> (typo) ^
> > avoid wrapping the document being dumped completely (size of the
> > non-wrapped
> > line before the patch was limited by MAX_LINE = 1024 by default).
> > Specifying
> > -dont-wrap-pre with interactive session enables wrapped lines (in PRE)
> > to
> > be marked as in source view - with '+' in normal view.
> > * Functionality of -force-empty-hrefless-a was cleaned up,
> > #ifndefs NO_EMPTY_HREFLESS_A and endif's were removed.
> > * Colorstyle changes are not emitted in HTML_end_element if me->skip_stack
> > was
> > >0 on entry to this function. This caused some glitches if
> > -force-empty-hrefless-a was enabled.
>
> Not just when -force-empty-hrefless-a was enabled, if I understand
> right. It was a more general problem with tagsoup recovery. Your
> description makes it look like the change is only relevant for
> '-force-empty-hrefless-a' users which isn't true.
Yes, the glitches could appear not only when '-force-empty-hrefless-a' was
enabled, but seems I never seen them (seems SET_SKIP_STACK is used for such
tags that don't have colors associated with them, like SELECT). May be html I
browsed didn't contain bugs.
> > * LYPrettySrc.c was slightly cleaned up, added support for lynx.cfg
> > reloading,
> > memory deallocation
> > * Possible bug in print_wwwfile_to_fd fixed - when quoting the page, seems
> > that
> > several '>' could be emitted on the long line formed by concatenating
> > ilnes
> > with LY_SOFT_NEWLINEs at the begining of them.
>
> See how it's growing and becoming less readable? I think you said it
> wouldn't, in connection with OUTGOING_MAIL_CHARSET discussion with Leonid.
Yes, it became less readable, but compared to 'split_line' or
'HText_appendCharacter it's very clear :)
> > General notes:
> > * This patch is designed to be applied after all 4 current Klaus' patches.
> > * Users that use lss-enabled lynx should consider on upgrading the lynx -
> > at
> > least those who use lynx for viewing huge files. By default, each line on
> > the
> > screen took 1K for colorstyles (on x86). With new approach colorstyle
> > changes
> > take 24 bytes (on x86) per line in average document. Old lynx with 900K
> > html
> > file loaded as startfile occupied 30M of virtual memory, lynx with patch
> > applied occupied 5 Mb on the same file. Old code is still accessible by
> > defining OLD_STYLECHANGE.
>
> So if this works without problems, the OLD_STYLECHANGE variant and symbol
> should be removed.
It works without problems - I used it for 1.5 monthes.
IMO it's better to leave them for several releases, so that other devers will
be able to see in which places the changes occur, how the macros are used,
etc. As you said, even a guy without inet connection should be able to modify
the code - such guys will be definitely unable to use rcs Tom has.
> > * I spent 4 hours debugging attempting to write/fix code that cleans up the
> > memory allocated in LYPrettySrc.c, but Lynx.leaks shows that I didn't
> > succeeded. I fear that leak detection stuff is broken - I checked/traced
> > the
> > code very carefully several times.
>
> I doubt that it is broken. I will check eventually (may take some time).
Don't pay a lot of attention to this. But I spend 4 hours with gdb.
Best regards,
-Vlad