Re: Unnecessary change

[Pavel Janík]

   From: Richard Stallman <address@hidden>
   Date: Thu, 20 Dec 2001 13:27:05 -0700 (MST)

   >    * emacsclient.c: Include "config.h", not <../src/config.h>.
   > 
   >     ! #ifdef HAVE_CONFIG_H
   >     ! #include "config.h"
   >     ! #endif
   > 
   > What was the reason for that change?  I don't see any advantage
   > in the change, and it is undesirable to change old code that works.
   > Was there some problem with the old code that I don't see?

When I learnt C, I was told that "config.h" form is used for programmer
written headers. <> is used for system-wide headers. gcc allows us to
overwrite the order of searching with <>, but this is not the case for
every compiler out there (some of them even append directories to the
search path instead of prepending as gcc does. I remember that I have met
such compiler at least once in my life on some commercial unix
system). Then ../src/config.h can easily be /usr/include/../src/config.h.
By unifying this, we can prevent these problems in the future - if there is
a bug, we can fix it at one place instead of hunting why we now use three
forms to include the same file:

pop.c:#include <../src/config.h>
profile.c:#include <config.h>
sorted-doc.c:#include "config.h"

Another reason is that I'm still newcomer to Emacs development team. Try to
think as one. You will read these three files. What do you think about the
programmer who wrote that? I'm very happy that Gerd accepted many of my
"unnecessary" changes (like unifying min/max macros which was defined about
40 times in the sources and others).

Yes, I agree that this change is not necessary (there was no bug-report
which this change fixed). If you do not agree with that change, you have
the full right to say so and remove that change.

When this e-mail was in my outgoing queue, I have also noticed this in
lib-src/ChangeLog:

1999-09-03  Richard Stallman  <address@hidden>

        * make-docfile.c: Include config.h not ../src/config.h.

But this was not the reason to commit my change. I just had a strong
feeling, that the new ("config.h") semantic is much better and error-proof
(what if we decide to do "mv src sources" in the future? Will we change
every occurrence of ../src/config.h?). Maybe you had the same feeling in
the past. Why not now?

I do not follow "passive" programming style (if it works, do not change
it), only when freeze/alpha/beta phase is in effect, sorry for that.

I originally sent this change (as part of a large patch which cleaned-up
lib-src) to Gerd on May, 5th with this comment:

--- cut here ---
To: Gerd Moellmann <address@hidden>
Subject: Emacs21: lib-src/* cleanup
Date: 05 May 2001 03:29:39 +0200

Hi Gerd,

I have spent some time on reading and understanding the code of emacsserver
and in the meantime I have cleaned up the lib-src directory, because there 
are some bad coding practices used. Here is the ChangeLog entry for
it. I know that this patch is not the "hey, this is really needed" one, but
it is really hard to understand the code if you use five different means of
including one file. Also original authors use some functions without
including the correct header file (ie. implicit declarations).

[...]

diff -urN emacs-21.0.103.orig/lib-src/emacsclient.c emacs-21.0.103/lib-src/emac
--- emacs-21.0.103.orig/lib-src/emacsclient.c   Sat May  5 02:47:05 2001
+++ emacs-21.0.103/lib-src/emacsclient.c        Sat May  5 02:47:40 2001
@@ -21,7 +21,9 @@
 
 
 #define NO_SHORTNAMES
-#include <../src/config.h>
+#ifdef HAVE_CONFIG_H

[...]
--- cut here ---

Yes, even on May 5th I thought that change was not necessary (*at that
time*, before the release). Now I think is the best time to include such
changes. I'm sorry that I had not consulted that before. I was waiting with
that change (yes, there are much important changes) and while rediffing my
cleaning patch against the updates done by Dave - see lib-src/ChangeLog:

2001-12-18  Dave Love  <address@hidden>

        * fakemail.c: Include "config.h", not <../src/config.h>.
        * emacsserver.c: Include "config.h", not <../src/config.h>.

I thought I can happily move that patch from my tree to save my time
rediffing it in the future. Dave, were your reasons different then mine?
-- 
Pavel Janík

printk("Penguin %d is stuck in the bottle.\n", i);
                  -- 2.0.38 arch/sparc/kernel/smp.c