lynx-dev coding style, ifdefs

[Klaus Weide]

On Fri, 15 Oct 1999, Vlad Harchev wrote:
>  I tried to say that I don't do boolean expressions optimization in
> preprocessor's conditionals yet:)

I still don't understand - hope it isn't important.
You are a programmer (or hacker, or whatever - a human at any rate),
why should you do a programs job?

> > > Probably indenting could help. 
> > 
> > I don't think so.  If you indent the same thing differently from the style
> > used in most of the code, it may just get even more confusing.
> 
>  As you should have noted, a lot of my changes are 'minimalistic' - they
> add lines - mostly conditional statements around some code, but don't indent
> that code. Sample:
> 
> was:
> 
>       foo(1); 
>       foo(2);
> 
> after patch:
> 
> #ifndef NO_BAR
>       if (bar_flag) {
> #endif
>       foo(1);
>       foo(2);
> #ifndef NO_BAR
>       };
> #endif
> 
> instead of properly indented:
> 
> #ifndef NO_BAR
>         if (bar_flag) {
> #endif
>             foo(1);
>             foo(2);
> #ifndef NO_BAR
>         };
> #endif

The fact that indentation is "right" for one set of preprocessor
macros and wrong for another should show you that you shouldn't do it
this way at all.

Make it
 
  #ifndef NO_BAR
          if (bar_flag) {
              foo(1);
              foo(2);
          };
  #else
          foo(1);
          foo(2);
  #endif

or better

  #ifdef NO_BAR
          foo(1);
          foo(2);
  #else
          if (bar_flag) {
              foo(1);
              foo(2);
          };
  #endif

or even better

  #ifdef BAR
          if (bar_flag) {
              foo(1);
              foo(2);
          };
  #else
          foo(1);
          foo(2);
  #endif

for readability, as long as foo(1), foo(2) is small.
Or, if you must make the condition itself alone dependent
on preprocessor conditions, write

  #ifndef NO_BAR
          if (bar_flag)
  #endif
          {
              foo(1);
              foo(2);
          };

But this kind of thing (if conditions that are conditional on
preprocessor stuff) is exactly what makes for unreadable code,
as soon as several variables and symbols are involved.  So the
best would be simply

          if (bar_flag) {
              foo(1);
              foo(2);
          };


> > > And anybody can ask me what something means.
> > 
> > That's a nice offer.  How long will it be valid?
> 
>   You fear that I will leave lynx development some day (I don't say you are 
> wrong)... Anyway, I will provide support (consultations).
> 
> > But it is irrelevent.  Nobody *should* have to ask you.
> 
>   Proper documentation of the code takes a lot of time. 

Writing some comments certainly takes less time than discussions on
lynx-dev and "providing consultations".

> > And not everybody *can* ask you.
> 
>   Why?

Because not everybody is subscribed to lynx-dev.  Because not
everybody has your address.  Because not everybody has Internet
access.  One should be able to use lynx, and have the source code,
in a usable form, without any of those things.

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

> > So, I would not have done it that way.  (If I had wanted a syntax
> > hightlighter like that, and had had an idea for it.)  I would have
> > not pursued it, or would have left SGML_character alone.
> 
>  I was a newbie to the lynx sources that time. I was not able to write the
> analogous parser similar to SGML.c (I was not able to spent the amount of time
> required to write such parser). 

But you could have started with a copy of SGML.c and made changes to that
copy.  No writing-from-scratch of existing stuff would have been required.
(And, to turn your argument around: you could have asked us.)

> But IMO SGML.c is not the hottest part of the
> sources tree - modifications are done by a very few people, and very seldom.

So should it be that way?

The next Vlad Harchev who wants to change something there (maybe only for
personal use) will have a harder time.

> So, programmers' efforts (regrading studying SGML.c) are less signtificant
> than the features/flexibility users get with syntax highlighter IMO.

Take this a step further, and you are basically saying that all that's
important is that users get features.  Don't bother about the coders.
Well if everyone before you had thought that way, you wouldn't have any
sort of more-or-less readable code to start playing with.  Maybe you
wouldn't have any free software.

> > Just since you were asking...  (well not literally.)
> > 
> > But I was not complaining that we now have a prettysrc mode.
> > I was complaining that it's hard to parse for the human reader.
> > And that may be exactly because you try to not change existing
> > lines, but you try to insert your new lines into to middle of existing
> > code in some clever way.
> 
>   I'm glad to hear that you like psrcmode as a user:)
>   As for places I chose for inserting new code - I hope you understand that I
> don't try to seem clever when I chose places - I chose the places in which
> it's more logically to insert the code (without breaking logic of the
> original, and allowing the new functionality to mimic the same logic).
> Sometimes I don't understand the logic in a whole...

> > And "I can explain it" is irrelevant.  Why do you think only "any
> > lynx developer" should be able to understand the code?
> 
>   Who else? What for? IMO we shouldn't make the lynx code "a tutorial on
> portable C programming" or "tutorial on programming console-based browsers" :)

Nobody suggested that... not that there's much chance of it happening.

> I prefer to spend less time and produce smaller/faster code.

If I read you right, you have already agreed that parts of your recent
code contributions need cleanup.  I hope you will find the time to do
that, even if you prefer to spend your time otherwise.


> > So your output (as it has ended up in the dev. code) is not a final
> > product, it is just an intermediate stage?
> 
>  All open-source products are subject to extending and enchancing. IMO
> denoting places that responsible for particular feature will help future
> developers to understand and enchance lynx code. 

That's what C has  /* COMMENTS */  for!

> As you saw, I leave both
> branches of conditionals (this is more than just providing marks) - yes, this
> can be considered redundant (but this more consistent way of utilizing cpp
> conditionals - isn't it?).

I shouldn't have to learn some special Vlad-preprocessing mode (by guessing -
it isn't spelled out anywhere) before I can make sense of it.

> > > As I remember, I posted cpp condition simplifier (as
> > > awk script, that remove 'conditionals with constant value' - just tell it 
> > > that
> > > NO_DUMP_WITH_BACKSPACES is always 0 and pass a code through it - thou' it
> > > can't handle expressions like x || y).
> > 
> > Why do you bring that up?  I am talking about the code that is distributed
> > in the dev.N .tar's and .zip's, not about anything one could hypothetically
> > make out of that with this or that tool.
> 
>  I meant that if these marks thierself will be considered redundant, they can
> be removed with the use of tools easily (rather than by hand).

So remove those that are in the way - whether you do it by hand or with
some fancy 'cpp condition simplifier' I don't care.

> > I am quite sure Tom doesn't need special macros for denoting where you
> > made changes.  Diff and so on make that unnecessary.
> 
>  After you apply patches, the history of patching is lost (thou' can be
> recreated by diffing the source trees).

No, he keeps all the old history (or, it is kept on sol.slcc.edu).
It could be made available.

> The use of cpp conditionals to mark places solves that problem. 

The problem you are trying to solve does not exist.

> As You and I said, in most places it's easier to
> remove the redundant conditionals with the use of tools.

Well I didn't say anything about tools...

Look, if you *really* aim at keeping a complete revision history of
code changes as conditional variants in the code - I shouldn't have
to tell you why that is a harebrained idea.  It doesn't even begin
to make sense.  If you mark only some changes that way, ad lib -
then it doesn't solve the problem (WDNE).

> > > And I'm sorry
> > > for OPT, OPT1 - these was an attempt for optimizations. I hope I will 
> > > clear
> > > them one day.

So do I. :)

>  Yes, this must be done. But I prefer to add more functionality than clean up
> the code.

Don't we all.

I'm still asking you to do your part to reduce #ifdef-hell.
(And complain when you see something from me that you think
should be doen differently).

   Klaus