Changes in process.c

Changes in process.c

[Eli Zaretskii]

People who make changes in process.c related to keyboard input and
wait_reading_process_output, please remember that there are two
branches in process.c, one for systems that support async
subprocesses, the other for those which don't (only MS-DOS in the
latter class).  Making changes only in one of them is going to break
something.

Btw, unless there are good reasons to have the new hold-keyboard
functions in process.c, I would suggest to move them to keyboard.c,
since this will eliminate the need to have 2 implementations of them.

split up process.c [was: Re: Changes in process.c]

[Dan Nicolaescu]

Eli Zaretskii <eliz@gnu.org> writes:

> People who make changes in process.c related to keyboard input and
> wait_reading_process_output, please remember that there are two
> branches in process.c, one for systems that support async
> subprocesses, the other for those which don't (only MS-DOS in the
> latter class).  Making changes only in one of them is going to break
> something.

This points to the ugliness of the situation.

Why don't we split the part for supporting MS-DOS into a different
file: process-no-subprocesses.c (or some better name)

That makes the file easier to read, less clunky, and problems easier
to catch with a simple grep.

Re: split up process.c [was: Re: Changes in process.c]

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Dan Nicolaescu <dann@gnu.org>
> Date: Thu, 08 Jul 2010 14:52:39 -0400
> 
> Why don't we split the part for supporting MS-DOS into a different
> file: process-no-subprocesses.c (or some better name)
> 
> That makes the file easier to read, less clunky, and problems easier
> to catch with a simple grep.

It will still leave two implementations of the same code.  If we want
to avoid that, the solution will need to be more radical than that.

Re: split up process.c [was: Re: Changes in process.c]

[Dan Nicolaescu]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: emacs-devel@gnu.org
>> From: Dan Nicolaescu <dann@gnu.org>
>> Date: Thu, 08 Jul 2010 14:52:39 -0400
>> 
>> Why don't we split the part for supporting MS-DOS into a different
>> file: process-no-subprocesses.c (or some better name)
>> 
>> That makes the file easier to read, less clunky, and problems easier
>> to catch with a simple grep.
>
> It will still leave two implementations of the same code.  

That's better than what we currently have.

There's already a precedent for multiple implementations for the same function: all of x_*

Re: split up process.c

[Glenn Morris]

Dan Nicolaescu wrote:

> There's already a precedent for multiple implementations for the
> same function: all of x_*

If you are talking about the platform-specific implementations of
x-create-frame, etc, that causes problems, and it is a stated goal to
merge them into a platform-independent version:

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=4402#16

Re: split up process.c

[Dan Nicolaescu]

Glenn Morris <rgm@gnu.org> writes:

> Dan Nicolaescu wrote:
>
>> There's already a precedent for multiple implementations for the
>> same function: all of x_*
>
> If you are talking about the platform-specific implementations of
> x-create-frame, etc, that causes problems, and it is a stated goal to

I am talking about the platform specific C functions, not DEFUNs
I am aware of the problems cause be DEFUNs.

Re: split up process.c [was: Re: Changes in process.c]

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Dan Nicolaescu <dann@gnu.org>
> Date: Thu, 08 Jul 2010 19:05:29 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Cc: emacs-devel@gnu.org
> >> From: Dan Nicolaescu <dann@gnu.org>
> >> Date: Thu, 08 Jul 2010 14:52:39 -0400
> >> 
> >> Why don't we split the part for supporting MS-DOS into a different
> >> file: process-no-subprocesses.c (or some better name)
> >> 
> >> That makes the file easier to read, less clunky, and problems easier
> >> to catch with a simple grep.
> >
> > It will still leave two implementations of the same code.  
> 
> That's better than what we currently have.

I did something more radical (revno 100767): unify the two branches of
process.c.

There's now only one function, wait_reading_process_output, which has
2 different implementations.  (I could easily have a single function
with two different bodies conditioned by `subprocesses', or I could
move the second implementation to msdos.c, if people prefer that.  But
both alternatives looked no cleaner, and the latter would even make
more maintenance headaches, IMO.)

Other than this single function, the rest is unified, at the cost of a
few "#ifdef subprocesses" here and there.  Not surprisingly, I found
and fixed a few bugs along the way...

Re: split up process.c

[Dan Nicolaescu]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: emacs-devel@gnu.org
>> From: Dan Nicolaescu <dann@gnu.org>
>> Date: Thu, 08 Jul 2010 19:05:29 -0400
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> Cc: emacs-devel@gnu.org
>> >> From: Dan Nicolaescu <dann@gnu.org>
>> >> Date: Thu, 08 Jul 2010 14:52:39 -0400
>> >> 
>> >> Why don't we split the part for supporting MS-DOS into a different
>> >> file: process-no-subprocesses.c (or some better name)
>> >> 
>> >> That makes the file easier to read, less clunky, and problems easier
>> >> to catch with a simple grep.
>> >
>> > It will still leave two implementations of the same code.  
>> 
>> That's better than what we currently have.
>
> I did something more radical (revno 100767): unify the two branches of
> process.c.

Thanks!

> There's now only one function, wait_reading_process_output, which has
> 2 different implementations.  (I could easily have a single function
> with two different bodies conditioned by `subprocesses', or I could
> move the second implementation to msdos.c, if people prefer that.  But
> both alternatives looked no cleaner, and the latter would even make
> more maintenance headaches, IMO.)

IMO it should go to msdos.c unless that requires other important code changes.

> Other than this single function, the rest is unified, at the cost of a
> few "#ifdef subprocesses" here and there.  Not surprisingly, I found
> and fixed a few bugs along the way...

BTW, it's kind of funny that msdos.c has MSDOS and subpprocesses #ifdefs.

Re: split up process.c

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Dan Nicolaescu <dann@gnu.org>
> Date: Sat, 10 Jul 2010 23:40:57 -0400
> 
> > There's now only one function, wait_reading_process_output, which has
> > 2 different implementations.  (I could easily have a single function
> > with two different bodies conditioned by `subprocesses', or I could
> > move the second implementation to msdos.c, if people prefer that.  But
> > both alternatives looked no cleaner, and the latter would even make
> > more maintenance headaches, IMO.)
> 
> IMO it should go to msdos.c unless that requires other important code changes.

I will defer to Stefan and Yidong for the decision.  Personally, I
think having a function with the same name on two different source
files will make maintenance a tad harder than it is when they are on
the same file.

> BTW, it's kind of funny that msdos.c has MSDOS and subpprocesses #ifdefs.

A long time ago I had plans to implement async subprocesses for the
MS-DOS port, and OTOH didn't dare to futz with non-MSDOS sources.

Thanks, I fixed this now (by reusing the similar definitions in
process.c).

Re: split up process.c

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>> > There's now only one function, wait_reading_process_output, which
>> > has 2 different implementations.  (I could easily have a single
>> > function with two different bodies conditioned by `subprocesses',
>> > or I could move the second implementation to msdos.c, if people
>> > prefer that.  But both alternatives looked no cleaner, and the
>> > latter would even make more maintenance headaches, IMO.)
>>
>> IMO it should go to msdos.c unless that requires other important code
>> changes.
>
> I will defer to Stefan and Yidong for the decision.  Personally, I
> think having a function with the same name on two different source
> files will make maintenance a tad harder than it is when they are on
> the same file.

I think the present situation is fine.