Re: [tcljava-dev] How to submit a code change (was: Patch for bug 121678)

[Christian K. <chr...@so...>]

Good morning,

> Humm, well I am not sure it is all that critical
> but I guess I don't have a big problem with it.
I would also not say that this is critical; its just
an incompatibility to Tcl.
> I would feel a lot better about it if we could
> add some test cases to Tcl 8.4 and then to Jacl,
> so that we knew this previously untested feature
> was going to be tested in the future.

> I still don't get how this relates to the
> other bits of the patch that add new
> exceptions when the file was not opened
> for writing and so on.

I you can give more than one distinct names for a channel,
the methods in the subclasses of Channel have no idea which
name they should print in the exception. This way you would
have the following:
  % read stdout
  channel "file1" wasn't opened for reading

Since I thought that an additional parameter "chanName" in
the methods Channel.read() and Channel.write() class isn't
a good idea, I did the same as Tcl does in tclIOCmd.c:
Let the command implementations check the channel before they
call the underlying channel operation, since they have the
original channel name available.

> Surely these conditions have test cases in the
> Tcl suite? If not, we really should
> add them to Tcl first. That way
> we can be sure Jacl and Tcl will
> not diverge in the future.

I just did a grep through the Tcl test suite and there is no
test which checks the name of a channel (as no good programmer
should rely on this).

> > > It seem like your patch does away with the
> > > StdChannel.getChanName() method. Would
> > > you not want to call setChanName(String)
> > > perhaps in the StdChannel() constructor?
> > I think that this change will not implement
> > the feature that the standard channels can be
> > named by two different names (e.g. standard input
> > with stdin and file0)...
> No, but it would make it so that you would
> not need to overload the getChanName()
> method, right?

I still don't understand this point. Especially as the patch
*removed* the overloaded StdChannel.getChanName() method.

Greetings, Krischan
-- 
Christian Krone, SQL Datenbanksysteme GmbH
Mail mailto:chr...@so...