on 4/12/00 1:05 AM, Chris Meyer <[EMAIL PROTECTED]> wrote:
> The patch is available at
> http://www.ultratask.com/turbine-patches-2000041201.txt
Can you please post a patch without tabs in it? One part of playing in OS is
to maintain the code style that is present in the project. I personally am
very against tabs because they make things like patches really hard to read
since in email and web pages the indentation gets all messed up.
> + if ( false )
> + if ( data.getSession().getValue( "turbine.dialog.save" ) == null )
> + data.getSession().putValue( "turbine.dialog.save",
data.getParameters() );
> + // I don't like 'dialog.save'; anything better?
> + // This has to use the raw session because we
> may not have a user yet.
If you are going to send in patches, lets do patches that actually work and
or not questionable like that. Putting it in an if (false) block makes it
questionable to me.
Also, you name "Dialog" doesn't seem real clear to me. In fact, this whole
patch doesn't seem really well explained and it is modifying some core code
so I'm warry of it a bit. Could you please spend so more time on the exact
reason why this patch is needed and why you think that the data is shown in
the URL...I hate to be an idiot and a pain in the ass, but I have never seen
the behavior you are talking about so it is foreign to me...
So, request comes in:
http://www.server.com/servlets/Turbine/screen/Login
that gets redirected to:
http://www.server.com/servlets/Turbine/screen/Login?JServSessionIdservlets=2
34kjlj
user then types in their username/password and gets the next screen after
the Login screen...this could be defined in the nextscreen special variable.
When exactly would this information be transmitted to the user in the URL?
I can see that you are stuffing the parameters into the session in order to
maintain the state from the last request, but is that really needed?
> Note too that the changes are kind of scattered through the code. It is an
> easy thing to clean them up but since I don't have cvs write access I
> decided I would submit these changes as they stand so that others can see
> the concept before I go all out...
>
> Future changes would be: localize support for this stuff to a utility
> package or something, formalize the 'Dialog' concept a little more
> strongly, generalize so that other screens can request dialogs appear
> before they appear, and several other items.
Ok...do it right the first time and send in that patch. ;-) I don't want to
clutter the code up with things that need to be "future" changed...I want
them changed now. Lots and lots of people depend on this code on a daily
basis...if interfaces change or core code changes, I want to make sure that
it is really solid.
> I hope this is useful to someone and I look forward to evolving this patch
> into a truly general and useful system.
Evolve it now and lets get it into the code...
thanks,
-jon
--
Scarab -
Java Servlet Based - Open Source
Bug/Issue Tracking System
<http://scarab.tigris.org/>
------------------------------------------------------------
To subscribe: [EMAIL PROTECTED]
To unsubscribe: [EMAIL PROTECTED]
Problems?: [EMAIL PROTECTED]
