Dear Damon,
thank you for the reply! It is a pleasure that someone looked it up.
Am 29.05.2012 17:26, schrieb Damon Courtney:
>> Comments:
>> load_response:
>> - insured that output array exists
>
> Just to note, you are breaking compatibility here. Previously one could rely
> on ![info exists response] to test whether any response had, in fact, been
> loaded. I'm not a backward compatibility nut, but change for the sake of
> change is bad. Is there a compelling reason for this?
No. I just had the sequence:
load_response
parray response
which failed if there was no response.
So I added that without thinking to far.
There is no big reason for it.
IMHO there is a lack of documentation here.
>> form.tcl:
>> - return -> return ""
>> May have side effects otherwise, as last result is returned
>
> There is no difference between [return] and [return ""]. A return with no
> arguments returns an empty string.
Correct, sorry.
> Don't use the uuid package. There's no need for it.
> Just use a simple, unique number within the request.
> There's no need for the overhead of generating a UUID (small though it
may be),
> and there's definitely no need to add a requirement for tcllib to Rivet.
> If you want to ship Rivet with tcllib included, I could be persuaded
for that,
> but don't make me get tcllib just to get a unique name for an HTML
form element.
I agree.
> ## If you don't mind creating a variable in the request namespace
> autogen_[incr ::request::__formElemCount]
>
> OR
>
> autogen_[info cmdcount]
>
> OR
>
> autogen_[clock microseconds]
>
> would all work just fine without the requirement of another package.
The uuid dependency was not introduced by myself.
I made this change to make the form package running for me, as I have
currently no uuid package.
I would prefer something more opaque to the user.
Thus modify method radiobuttons/checkboxes to use the name and an
increasing number as id and document that.
I would also prefer to have only an id generated if there is a special
parameter. Example:
my_form checkbox check -id #auto
which would generate the id check_[info cmdcount]
But that is another discussion. Not my building site.
I would encourrage Massimo to use your solution.
> Being that there are many bug fixes but also MANY incompatibilities in this
> version of
> the form package, we might consider moving these changes to a
form2.tcl file and
> giving it a [package provide form 2.0] label.
> This would leave the old one in place, and anyone running into
compatibility issues
> could simply change [package require form] to [package require form 1.0] to
> get the
> old one and keep running without all the pain in the ass.
>
> I'm not disagreeing with the changes here.
> They look good, and they do fix bugs.
> But give people a temporary migration path.
The only purpose is fixing bugs (52650, 52909) and I am not in favour
calling it forms2.
For me, the uuid-issue is heavy, but that was not me.
As the current form package has security issues due to the list handling
(you may easily crash it by special user input:
load_response
form my_form
form start
my_form radiobuttons radio -values {1 2 3}
form end
Called with
http://test.rvt?radio=%7B
will cause a runtime error, as the data is no valid list and it is
accessed via "lsearch" which causes a crash. )
there is action needed (See my monologues on bugs 52650, 52653, 52909).
So I agreed with Massimo to use the magic __$name indexes to indicate
lists. I was not for this method but it is the most compatible way.
So this is my implementation proposal.
I agreed with Massimo that he will refine it as we did with the
rivet::entity package.
Damon, if you have any proposals what to change, please post them and
otherwise check the package if it works for you.
IMHO there are only incompatibilities introduced which are necessary.
When I started with the form package, the default mechanism did not work
at all due to a variable name clash...
Thank you all,
Harald
---------------------------------------------------------------------
To unsubscribe, e-mail: rivet-dev-unsubscr...@tcl.apache.org
For additional commands, e-mail: rivet-dev-h...@tcl.apache.org
