Re: lynx-dev megapatch to dev.10 - and a rant

[Klaus Weide]

On Thu, 14 Oct 1999, Vlad Harchev wrote:

>  I tried to apply your patch to dev10. It fails to compile since
> 'src/TRSTable.c' includes non-existing file 'AC.c'. Can you make it available
> (if you make that file available separately, I will be even more thankful to
> you).

Very sorry about that. It is only one function that I moved to a separate
file for testing, and then completely forgot about.  The contents are
appended, if should just replace the `#include "AC.c"' in TRSTable.c.

>  As for table rendering - I started thinking on implementing it completely,
> using something like w3m algorithms.

Have you looked at w3m's algorithms?  (I haven't looked at any.)
If yes a summary of the approach would be interesting - just out of
curiosity.

   ------------

Btw Vlad, while doing that merging of my older changes I found
that there are some pieces of code that have become IMO completely
unreadable, and some of your changes are foremost...
Some examples

(1) in HText_appendCharacter

    if (IsSpecialAttrChar(ch) && ch != LY_SOFT_NEWLINE) {
#if !defined(USE_COLOR_STYLE) || !defined(NO_DUMP_WITH_BACKSPACES)
        if (line->size >= (MAX_LINE-1)) return;
#if defined(USE_COLOR_STYLE) && !defined(NO_DUMP_WITH_BACKSPACES)
        if (with_backspaces && HTCJK==NOCJK && !text->T.output_utf8) {
#endif
        if (ch == LY_UNDERLINE_START_CHAR) {
            .....

Does *anybody* get a clue, by looking at this, under which circumstances 
the following section gets executed and under which it doesn't?
With the double negatives combined with other flags that then control
other conditions etc., I can stare at it for 20 minutes and still not get
it.

(2) in form_getstr

            case LTARROW:       /* 1999/04/14 (Wed) 15:01:33 */
                if (MyEdit.pos == 0 && repeat == -1) {
                    int c = YES;    /* Go back immediately if no changes */
#ifndef NO_NONSTICKY_INPUTS
                    if (sticky_inputs
                     && !textfield_stop_at_left_edge)
#endif
                    if (strcmp(MyEdit.buffer, value)) {
                        c = HTConfirmDefault(PREV_DOC_QUERY, NO);
                    }
                    if (c == YES) {
#ifndef NO_NONSTICKY_INPUTS
                        if (textfield_stop_at_left_edge)
                            goto again;
#endif
                        return(ch);
                    } else {

Again, I don't get it.  In fact it seems to be wrong, and seems to
have the result that now - with the set of macros I have compiled
with *which do not include anything about `STICKY'* - I get *trapped*
in form fields without a prompt to escape.  But the logic is too
convoluted to see easily what's supposed to happen.

(3) a lot in SGML.c - 'nuff said.

This proliferation of newe proprocessor macros to control some
micro-feature is A Very Bad Thing.  Particularly when they are
logiccal negatives and are used in complex logical combinations
with other conditions, preprocessor and runtime.

I was hesitant to write this, since no doubt I have done my share to
make lynx code harder to understand.  Glass house and stones etc.
Still it has to be said.  If you find I have done the same you can
rant about it, too. :)

So can you please do something to make your addtions more
understandable.  Particularly the examples above.  (And check
whether that sticky stuff really does what it's supposed to,
not just in your configuration.)  

Some of those alternatives are just unnecessary.  You have to decide
whether a code alternative should be in the code or not.  If you
can't decide, and feel the need to include several variants, then
maybe the change isn't worth it at all.  I'm not talking about
features that it might make sense to disable or enable at compile
time, but those where it really doesn't make sense to provide
configurability (and I would count NO_DUMP_WITH_BACKSPACES, 
NO_NONSTICKY_INPUTS, and OPT,OPT1 in SGML.c here.  Either the code
savings if someone chosses not to compile it in are negligible,
or it's obvious that one alternative is _meant_ to be the better
one and should be used.)

Also I find the negative form you use for those new mini-features
("mini" in the sense that they don't affect much code, or shouldn't)
inconsiderate.  (That is under the asumption that it makes sense to have
conditional compilation at all.)  Now I have to do something special to
disable new stuff (that I probably don't want), i.e. define a NO_*
symbol - instead of making me define something only when I _do_ want it.


   Klaus

------------ "AC.c" follows ------------
PRIVATE void update_sumcols0 ARGS7(
    STable_cellinfo *,  sumcols,
    STable_rowinfo *,   lastrow,
    int,                pos,
    int,                len,
    int,                icell,
    int,                ispan,
    int,                allocated_sumcols)
{
    int i;
    if (len > 0) {
        int sumpos = pos;
        int prevsumpos = sumcols[icell + ispan].pos;
        int advance;
        if (ispan > 0) {
            if (lastrow->cells[icell].pos + len > sumpos)
                sumpos = lastrow->cells[icell].pos + len;
            if (sumcols[icell+ispan-1].pos + sumcols[icell+ispan-1].len > 
sumpos)
                sumpos = sumcols[icell+ispan-1].pos + 
sumcols[icell+ispan-1].len;
        }
        advance = sumpos - prevsumpos;
        if (advance > 0) {
            for (i = icell + ispan; i < allocated_sumcols; i++) {
                if (ispan > 0 && sumcols[i].colspan < -1) {
                    if (i + sumcols[i].colspan < icell + ispan) {
                        advance = sumpos - sumcols[i].pos;
                        if (i > 0)
                            advance = HTMAX(advance,
                                            sumcols[i-1].pos + sumcols[i-1].len
                                            - (sumcols[i].pos));
                        if (advance <= 0)
                            break;
                    }
                }
                if (sumcols[i].pos >= 0)
                    sumcols[i].pos += advance;
                else {
                    sumcols[i].pos = sumpos;
                    break;
                }
            }
        }
    }
}