* Re: fixing non-NS darwin (was: Re: your emacs/src/keyboard.h change)
2008-08-03 16:41 ` fixing non-NS darwin (was: Re: your emacs/src/keyboard.h change) Adrian Robert
@ 2008-08-04 0:23 ` YAMAMOTO Mitsuharu
2008-08-04 1:20 ` fixing non-NS darwin Dan Nicolaescu
2008-08-04 1:11 ` Dan Nicolaescu
2008-08-04 16:11 ` fixing non-NS darwin (was: Re: your emacs/src/keyboard.h change) Adrian Robert
2 siblings, 1 reply; 14+ messages in thread
From: YAMAMOTO Mitsuharu @ 2008-08-04 0:23 UTC (permalink / raw)
To: Adrian Robert; +Cc: Dan Nicolaescu, Emanuele Giaquinta, Emacs Development
>>>>> On Sun, 3 Aug 2008 12:41:22 -0400, Adrian Robert <adrian.b.robert@gmail.com> said:
> This is not defined anywhere in emacs, but there was this section in
> an earlier version of darwin.h:
> #if 0 /* Don't define DARWIN on Mac OS X because CoreFoundation.h uses
> it to distinguish Mac OS X from bare Darwin. */
> #ifndef DARWIN
> #define DARWIN 1
> #endif
> #endif
> Does anyone know where this IS defined? Also, I've been unable to
> find a version of CoreFoundation.h that makes the check referred to.
It's used in CoreFoundation.h in Mac OS X 10.1 - 10.3.
BTW, why the above comment was removed? The corresponding ChangeLog
entry only says:
2008-07-10 Dan Nicolaescu <dann@ics.uci.edu>
* unexec.c:
* s/vms.h:
* s/usg5-4-2.h:
* s/sol2-5.h:
* s/freebsd.h:
* s/darwin.h: Remove dead code.
The comments like this should be left for future reference so people
may not make the same mistake again.
YAMAMOTO Mitsuharu
mituharu@math.s.chiba-u.ac.jp
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: fixing non-NS darwin
2008-08-04 0:23 ` YAMAMOTO Mitsuharu
@ 2008-08-04 1:20 ` Dan Nicolaescu
2008-08-04 2:03 ` YAMAMOTO Mitsuharu
0 siblings, 1 reply; 14+ messages in thread
From: Dan Nicolaescu @ 2008-08-04 1:20 UTC (permalink / raw)
To: YAMAMOTO Mitsuharu; +Cc: Adrian Robert, Emanuele Giaquinta, Emacs Development
YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:
> >>>>> On Sun, 3 Aug 2008 12:41:22 -0400, Adrian Robert <adrian.b.robert@gmail.com> said:
>
> > This is not defined anywhere in emacs, but there was this section in
> > an earlier version of darwin.h:
>
> > #if 0 /* Don't define DARWIN on Mac OS X because CoreFoundation.h uses
> > it to distinguish Mac OS X from bare Darwin. */
> > #ifndef DARWIN
> > #define DARWIN 1
> > #endif
> > #endif
>
> > Does anyone know where this IS defined? Also, I've been unable to
> > find a version of CoreFoundation.h that makes the check referred to.
>
> It's used in CoreFoundation.h in Mac OS X 10.1 - 10.3.
>
> BTW, why the above comment was removed?
Because it looked like dead code, and the description was not something
that made it clear that it was needed as documentation, nor was the
structure of the code. There is a proper place now to properly document
macros: admin/CPP-DEFINES.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: fixing non-NS darwin
2008-08-04 1:20 ` fixing non-NS darwin Dan Nicolaescu
@ 2008-08-04 2:03 ` YAMAMOTO Mitsuharu
2008-08-04 2:26 ` Dan Nicolaescu
0 siblings, 1 reply; 14+ messages in thread
From: YAMAMOTO Mitsuharu @ 2008-08-04 2:03 UTC (permalink / raw)
To: Dan Nicolaescu; +Cc: Adrian Robert, Emanuele Giaquinta, Emacs Development
>>>>> On Sun, 03 Aug 2008 18:20:05 -0700, Dan Nicolaescu <dann@ics.uci.edu> said:
>> > This is not defined anywhere in emacs, but there was this section in
>> > an earlier version of darwin.h:
>>
>> > #if 0 /* Don't define DARWIN on Mac OS X because CoreFoundation.h uses
>> > it to distinguish Mac OS X from bare Darwin. */
>> > #ifndef DARWIN
>> > #define DARWIN 1
>> > #endif
>> > #endif
>>
>> > Does anyone know where this IS defined? Also, I've been unable to
>> > find a version of CoreFoundation.h that makes the check referred to.
>>
>> It's used in CoreFoundation.h in Mac OS X 10.1 - 10.3.
>>
>> BTW, why the above comment was removed?
> Because it looked like dead code, and the description was not something
> that made it clear that it was needed as documentation, nor was the
> structure of the code.
It's quite a common way in the Emacs code to comment out with #if 0
with leaving some explanation about why it is disabled. Especially
for the case that people might make the same mistake again in future
unconsciously if that part were completely removed.
Cleanup tasks are usually tedious, and thus would be much appreciated
if done carefully and appropriately. But as they are also inherently
optional, not appreciated if done less carefully or unnecessarily
aggressively as in the case of MULTI_KBOARD.
YAMAMOTO Mitsuharu
mituharu@math.s.chiba-u.ac.jp
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: fixing non-NS darwin
2008-08-04 2:03 ` YAMAMOTO Mitsuharu
@ 2008-08-04 2:26 ` Dan Nicolaescu
2008-08-04 3:49 ` YAMAMOTO Mitsuharu
0 siblings, 1 reply; 14+ messages in thread
From: Dan Nicolaescu @ 2008-08-04 2:26 UTC (permalink / raw)
To: YAMAMOTO Mitsuharu; +Cc: Adrian Robert, Emanuele Giaquinta, Emacs Development
YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:
> >>>>> On Sun, 03 Aug 2008 18:20:05 -0700, Dan Nicolaescu <dann@ics.uci.edu> said:
>
> >> > This is not defined anywhere in emacs, but there was this section in
> >> > an earlier version of darwin.h:
> >>
> >> > #if 0 /* Don't define DARWIN on Mac OS X because CoreFoundation.h uses
> >> > it to distinguish Mac OS X from bare Darwin. */
> >> > #ifndef DARWIN
> >> > #define DARWIN 1
> >> > #endif
> >> > #endif
> >>
> >> > Does anyone know where this IS defined? Also, I've been unable to
> >> > find a version of CoreFoundation.h that makes the check referred to.
> >>
> >> It's used in CoreFoundation.h in Mac OS X 10.1 - 10.3.
> >>
> >> BTW, why the above comment was removed?
>
> > Because it looked like dead code, and the description was not something
> > that made it clear that it was needed as documentation, nor was the
> > structure of the code.
>
> It's quite a common way in the Emacs code to comment out with #if 0
> with leaving some explanation about why it is disabled. Especially
> for the case that people might make the same mistake again in future
> unconsciously if that part were completely removed.
So? Everyone is aware of that.
But that was absolutely not the case in this particular instance. The
comments and code did not make the intention clear for the casual reader.
If you actually want to contribute something positive, please add the
missing documentation and fix the code.
> Cleanup tasks are usually tedious, and thus would be much appreciated
> if done carefully and appropriately. But as they are also inherently
> optional, not appreciated if done less carefully or unnecessarily
> aggressively as in the case of MULTI_KBOARD.
Did you just wait for an occasion to attack me on an unrelated item? Grow up!
Sorry, I am not interested in your badgering, and given you past
abhorrent behavior you are not in a position to lecture anyone.
Do I have to repeat it yet again? STOP WAISTING MY TIME!
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: fixing non-NS darwin
2008-08-04 2:26 ` Dan Nicolaescu
@ 2008-08-04 3:49 ` YAMAMOTO Mitsuharu
2008-08-04 4:02 ` Dan Nicolaescu
0 siblings, 1 reply; 14+ messages in thread
From: YAMAMOTO Mitsuharu @ 2008-08-04 3:49 UTC (permalink / raw)
To: Dan Nicolaescu; +Cc: Adrian Robert, Emanuele Giaquinta, Emacs Development
>>>>> On Sun, 03 Aug 2008 19:26:24 -0700, Dan Nicolaescu <dann@ics.uci.edu> said:
>> It's quite a common way in the Emacs code to comment out with #if 0
>> with leaving some explanation about why it is disabled. Especially
>> for the case that people might make the same mistake again in
>> future unconsciously if that part were completely removed.
> So? Everyone is aware of that. But that was absolutely not the case
> in this particular instance. The comments and code did not make the
> intention clear for the casual reader.
Would casual readers care about admin/CPP-DEFINES? Anyway, I did not
expect casual readers would touch the code in the CVS, even for
cleanups.
> If you actually want to contribute something positive, please add
> the missing documentation and fix the code.
It's not just a local problem. I'm concerned about possible removals
of other important comments in your past/future cleanups. It's really
inefficient if every developer has to check the excessive removal of
comments in (inherently optional) cleanups.
>> Cleanup tasks are usually tedious, and thus would be much
>> appreciated if done carefully and appropriately. But as they are
>> also inherently optional, not appreciated if done less carefully or
>> unnecessarily aggressively as in the case of MULTI_KBOARD.
> Did you just wait for an occasion to attack me on an unrelated item?
> Grow up!
Unrelated? I'd rather think these troubles come from the same
mindset. Did I wait? I hoped such kind of troubles wouldn't happen
again, but actually I could anticipate that would.
YAMAMOTO Mitsuharu
mituharu@math.s.chiba-u.ac.jp
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: fixing non-NS darwin
2008-08-04 3:49 ` YAMAMOTO Mitsuharu
@ 2008-08-04 4:02 ` Dan Nicolaescu
2008-08-04 9:08 ` Thien-Thi Nguyen
0 siblings, 1 reply; 14+ messages in thread
From: Dan Nicolaescu @ 2008-08-04 4:02 UTC (permalink / raw)
To: YAMAMOTO Mitsuharu; +Cc: Adrian Robert, Emanuele Giaquinta, Emacs Development
YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:
> >>>>> On Sun, 03 Aug 2008 19:26:24 -0700, Dan Nicolaescu <dann@ics.uci.edu> said:
>
> >> It's quite a common way in the Emacs code to comment out with #if 0
> >> with leaving some explanation about why it is disabled. Especially
> >> for the case that people might make the same mistake again in
> >> future unconsciously if that part were completely removed.
>
> > So? Everyone is aware of that. But that was absolutely not the case
> > in this particular instance. The comments and code did not make the
> > intention clear for the casual reader.
>
> Would casual readers care about admin/CPP-DEFINES?
Of course, that is why it exists.
> > If you actually want to contribute something positive, please add
> > the missing documentation and fix the code.
>
> It's not just a local problem. I'm concerned about possible removals
> of other important comments in your past/future cleanups. It's really
> inefficient if every developer has to check the excessive removal of
> comments in (inherently optional) cleanups.
As usual you are making a mountain out of a molehill and making a
racket _again_ from nothing.
Sorry, this is beyond silly. So again: QUIT WAISTING MY TIME!
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: fixing non-NS darwin
2008-08-04 4:02 ` Dan Nicolaescu
@ 2008-08-04 9:08 ` Thien-Thi Nguyen
2008-08-04 11:53 ` Dan Nicolaescu
0 siblings, 1 reply; 14+ messages in thread
From: Thien-Thi Nguyen @ 2008-08-04 9:08 UTC (permalink / raw)
To: Dan Nicolaescu; +Cc: Emacs Development
() Dan Nicolaescu <dann@ics.uci.edu>
() Sun, 03 Aug 2008 21:02:29 -0700
> Would casual readers care about admin/CPP-DEFINES?
Of course, that is why it exists.
[...]
So again: QUIT WAISTING MY TIME!
Dan, why don't you DTRT and add a blurb to admin/CPP-DEFINES?
This small effort will save everyone time.
Alternatively, amend the ChangeLog to bring it up to standard.
thi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: fixing non-NS darwin
2008-08-04 9:08 ` Thien-Thi Nguyen
@ 2008-08-04 11:53 ` Dan Nicolaescu
0 siblings, 0 replies; 14+ messages in thread
From: Dan Nicolaescu @ 2008-08-04 11:53 UTC (permalink / raw)
To: Thien-Thi Nguyen; +Cc: Emacs Development
Thien-Thi Nguyen <ttn@gnuvola.org> writes:
> () Dan Nicolaescu <dann@ics.uci.edu>
> () Sun, 03 Aug 2008 21:02:29 -0700
>
> > Would casual readers care about admin/CPP-DEFINES?
>
> Of course, that is why it exists.
>
> [...]
>
> So again: QUIT WAISTING MY TIME!
>
> Dan, why don't you DTRT and add a blurb to admin/CPP-DEFINES?
> This small effort will save everyone time.
This thread started because I didn't know the correct meaning of that
macro, not did the current maintainer...
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: fixing non-NS darwin
2008-08-03 16:41 ` fixing non-NS darwin (was: Re: your emacs/src/keyboard.h change) Adrian Robert
2008-08-04 0:23 ` YAMAMOTO Mitsuharu
@ 2008-08-04 1:11 ` Dan Nicolaescu
2008-08-04 16:11 ` fixing non-NS darwin (was: Re: your emacs/src/keyboard.h change) Adrian Robert
2 siblings, 0 replies; 14+ messages in thread
From: Dan Nicolaescu @ 2008-08-04 1:11 UTC (permalink / raw)
To: Adrian Robert; +Cc: Emanuele Giaquinta, Emacs Development
Adrian Robert <adrian.b.robert@gmail.com> writes:
> On Aug 3, 2008, at 11:59 AM, Dan Nicolaescu wrote:
>
> > While at it, can you please get rid of -DMAC_OSX from src/s/darwin.h?
>
> After some discussion with other developers, I think there will need
> to be some kind of #define there, analogous to WINDOWSNT in s/ms-
> w32.h, GNU_LINUX in s/gnu-linux.h, etc.. MAC_OSX is wrong because it
> could be a non-OS X Darwin system. DARWIN is wrong because that is
> apparently defined by the system includes and/or compiler on non-OS X
> Darwin (see below). I don't think just the BSD4_2 that's in darwin.h
> is specific enough.
>
> I am thinking something like DARWIN_BASED_OS?
Is there something that the compiler defines by default? If there is,
it would be better to use that instead.
If not, I'd vote for DARWIN_OS because is shorter.
> > And please add documentation to admin/CPP-DEFINES for the DARWIN
> > macro.
>
> This is not defined anywhere in emacs, but there was this section in
> an earlier version of darwin.h:
>
> #if 0 /* Don't define DARWIN on Mac OS X because CoreFoundation.h uses
> it to distinguish Mac OS X from bare Darwin. */
> #ifndef DARWIN
> #define DARWIN 1
> #endif
> #endif
>
> Does anyone know where this IS defined? Also, I've been unable to
> find a version of CoreFoundation.h that makes the check referred to.
So what should happen to the code that checks for DARWIN ?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: fixing non-NS darwin (was: Re: your emacs/src/keyboard.h change)
2008-08-03 16:41 ` fixing non-NS darwin (was: Re: your emacs/src/keyboard.h change) Adrian Robert
2008-08-04 0:23 ` YAMAMOTO Mitsuharu
2008-08-04 1:11 ` Dan Nicolaescu
@ 2008-08-04 16:11 ` Adrian Robert
2008-08-04 17:31 ` fixing non-NS darwin Dan Nicolaescu
2008-08-05 9:21 ` fixing non-NS darwin (was: Re: your emacs/src/keyboard.h change) YAMAMOTO Mitsuharu
2 siblings, 2 replies; 14+ messages in thread
From: Adrian Robert @ 2008-08-04 16:11 UTC (permalink / raw)
To: Adrian Robert; +Cc: Dan Nicolaescu, Emacs Development
On Aug 3, 2008, at 12:41 PM, Adrian Robert wrote:
>
> On Aug 3, 2008, at 11:59 AM, Dan Nicolaescu wrote:
>
>> This code in emacs.c is not right:
>>
>> #if defined (NS_IMPL_COCOA)
>> if (!initialized)
>> unexec_init_emacs_zone ();
>> #endif
>>
>> it should be done on all MacOSX configs.
>
> I've been looking at this, and made a diff of before and after
> remove-carbon to see what else might have been affected, since I
> never realized that the undef MAC_OSX wasn't working.
OK, the above should be fixed. (DARWIN_OS is set in darwin.h and used
to indicate on OS X or pure Darwin; unexmacosx.c should be at some
point renamed to unexdarwin.c.)
I've also gone through the removal diff and wanted to ask about the
bit below since it may be something that all terms should do:
Index: atimer.c
===================================================================
RCS file: /sources/emacs/emacs/src/atimer.c,v
retrieving revision 1.28
retrieving revision 1.27
diff -u -r1.28 -r1.27
--- atimer.c 27 Jul 2008 18:24:40 -0000 1.28
+++ atimer.c 14 May 2008 07:49:08 -0000 1.27
@@ -368,7 +368,9 @@
t = atimers;
atimers = atimers->next;
+#ifndef MAC_OSX
t->fn (t);
+#endif
if (t->type == ATIMER_CONTINUOUS)
{
@@ -380,6 +382,10 @@
t->next = free_atimers;
free_atimers = t;
}
+#ifdef MAC_OSX
+ /* Fix for Ctrl-G. Perhaps this should apply to all platforms.
*/
+ t->fn (t);
+#endif
EMACS_GET_TIME (now);
}
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: fixing non-NS darwin
2008-08-04 16:11 ` fixing non-NS darwin (was: Re: your emacs/src/keyboard.h change) Adrian Robert
@ 2008-08-04 17:31 ` Dan Nicolaescu
2008-08-05 9:21 ` fixing non-NS darwin (was: Re: your emacs/src/keyboard.h change) YAMAMOTO Mitsuharu
1 sibling, 0 replies; 14+ messages in thread
From: Dan Nicolaescu @ 2008-08-04 17:31 UTC (permalink / raw)
To: Adrian Robert; +Cc: Emacs Development
Adrian Robert <adrian.b.robert@gmail.com> writes:
> I've also gone through the removal diff and wanted to ask about the
> bit below since it may be something that all terms should do:
>
> Index: atimer.c
> ===================================================================
> RCS file: /sources/emacs/emacs/src/atimer.c,v
> retrieving revision 1.28
> retrieving revision 1.27
> diff -u -r1.28 -r1.27
> --- atimer.c 27 Jul 2008 18:24:40 -0000 1.28
> +++ atimer.c 14 May 2008 07:49:08 -0000 1.27
> @@ -368,7 +368,9 @@
>
> t = atimers;
> atimers = atimers->next;
> +#ifndef MAC_OSX
> t->fn (t);
> +#endif
>
> if (t->type == ATIMER_CONTINUOUS)
> {
> @@ -380,6 +382,10 @@
> t->next = free_atimers;
> free_atimers = t;
> }
> +#ifdef MAC_OSX
> + /* Fix for Ctrl-G. Perhaps this should apply to all
> platforms. */
> + t->fn (t);
> +#endif
>
> EMACS_GET_TIME (now);
> }
Is there evidence that this fixes a bug? Otherwise it would be very
likely against the feature freeze rules to check in something like
this.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: fixing non-NS darwin (was: Re: your emacs/src/keyboard.h change)
2008-08-04 16:11 ` fixing non-NS darwin (was: Re: your emacs/src/keyboard.h change) Adrian Robert
2008-08-04 17:31 ` fixing non-NS darwin Dan Nicolaescu
@ 2008-08-05 9:21 ` YAMAMOTO Mitsuharu
1 sibling, 0 replies; 14+ messages in thread
From: YAMAMOTO Mitsuharu @ 2008-08-05 9:21 UTC (permalink / raw)
To: Adrian Robert; +Cc: Emacs Development
>>>>> On Mon, 4 Aug 2008 12:11:31 -0400, Adrian Robert <adrian.b.robert@gmail.com> said:
> I've also gone through the removal diff and wanted to ask about the
> bit below since it may be something that all terms should do:
Well, careful (as opposed to casual) readers would pay attention to
such a comment (not written by me, IIRC).
A related discussion is found at:
http://lists.gnu.org/archive/html/emacs-pretest-bug/2004-04/msg00141.html
Still I can't clearly explain why it is ok to do a longjmp and not
execute the call to set_alarm from alarm_signal_handler. But this
change does fix a particular problem and at least does not make things
worse, and it has been working well with Carbon Emacs 22 for a long
time.
YAMAMOTO Mitsuharu
mituharu@math.s.chiba-u.ac.jp
> Index: atimer.c
> ===================================================================
> RCS file: /sources/emacs/emacs/src/atimer.c,v
> retrieving revision 1.28
> retrieving revision 1.27
> diff -u -r1.28 -r1.27
> --- atimer.c 27 Jul 2008 18:24:40 -0000 1.28
> +++ atimer.c 14 May 2008 07:49:08 -0000 1.27
> @@ -368,7 +368,9 @@
>
> t = atimers;
> atimers = atimers->next;
> +#ifndef MAC_OSX
> t-> fn (t);
> +#endif
>
> if (t->type == ATIMER_CONTINUOUS)
> {
> @@ -380,6 +382,10 @@
> t->next = free_atimers;
> free_atimers = t;
> }
> +#ifdef MAC_OSX
> + /* Fix for Ctrl-G. Perhaps this should apply to all platforms.
> */
> + t->fn (t);
> +#endif
>
> EMACS_GET_TIME (now);
> }
^ permalink raw reply [flat|nested] 14+ messages in thread