Thanks a lot for the comments Leslie.  Some comments below:

> Please try to split up the code into multiple files, preferably
> one per class. It's cumbersome to review these huge chunks of code.

Sorry about that, will keep that in mind.

> Why is it necessary to supply this variable to the next method(s)?

This way I can do things like provide the current-view when creating
fields or presentations (for example, in my login widget, I have a
on-change method that curries the *current-view* into the callback).

> (defgeneric render-presentation (obj value) ...)
>
> That seems too limited, I think we also need to be able to specialize
> on the view, the form widget and the view field name.

The thing is, at that abstraction level, the presentation has no
knowledge of the view field name or the widget that is using this
presentation.  I suppose we could have it take in a view field name or
widget, but is there an example of a scenario where you could see
rendering the presentation differently based on the field or the
widget using the presentation?  I just didn't want to start having
widget rendering and field rendering getting mixed into the logic of
presentation rendering.

>   (validator
>    :initform nil
>    :initarg :validator
>    :accessor stateful-field-validator
>
> Don't forget that it should be possible to specify multiple
> validators.

I hadn't thought of that - you were thinking this should be allowed at
the field level as well as the view level?

>   (data-class
>    :initform nil
>    :initarg :data
>    :accessor stateful-view-data-class
>
> Why not always derive the data class store via CLASS-STORE?

It would be my preference to do that (you mean by doing (object-store
data-object)?  If not, I'm not entirely sure what you mean).  I put
that there because it seemed like dataform currently keeps around a
class-store, and I wasn't sure if there was a use case for this (maybe
having the same data-object in multiple stores?).  If there isn't a
reason for this, I'll get rid of it.

>  (declaim (inline current-view))
>  (defun current-view ()
>    *current-view*)
>
> Why this? To provide some abstraction? You probably want a macro
> here to make it SETFable too.

Oops, that was something I was messing around with that I meant to
take out before committing.

>  (defun stringified-slot-names (obj)
>
> Is this really necessary?

Nope, and I just noticed it isn't used anywhere - I was relying on it
before I figured out a better way to do slot name comparisons between
data objects and views - I'll get rid of it.

>      ;; This error checking looks a bit stupid.
>      (when (slot-in-object-p 'data-object view-data)
>        (error "Tried to map a data-model to a view when the data-model has a 
> slot called
> 'data-object', which is reserved."))
>      (when (slot-in-object-p 'data-class view-data)
>        (error "Tried to map a data-model to a view when the data-model has a 
> slot called
> 'data-class', which is reserved."))
>      (when (slot-in-object-p 'validator view-data)
>        (error "Tried to map a data-model to a view when the data-model has a 
> slot called
> 'validator', which is reserved."))
>
> Yes, why not rely on packages here and allow these slot names?
>

The reason I'm not just relying on packages is because the mapping
being done in the views is package agnostic.  So, for example, if you
create a view with slots called "name, address" in a package called
"test" and then your data object that you are tying to the view is in
a package called "test1", slots called "name" and "address" on the
data object will still be mapped to the name/address slots in the
view.  Thus, without these checks, if you have a data-object that has
a slot called "validator" in any package, and you have currently not
assigned anything to the validator slot for the view, the
initialize-instance will create a default field for the validator slot
for the view.  This is why slot-in-object-p forces the slot names to a
string before doing the comparison.

I could rely on packages here, but it seemed too constraining to
assume that data objects and their corresponding views will always be
in the same package, but I don't have a very strong opinion on that.

>> 3) One thing I wasn't sure about was whether views should keep state
>> or not.
>
> But in the following text you talk about view fields, not views.
>
> Do you mean view fields above, or both?
>

Sorry, meant view fields only.  Basically, the current implementation.

>> 5) I started on something to allow creating presentations on the fly
>> using lambdas, but was foiled because I'm not sure how to tell if a
>> form presentation created in a lambda is a form presentation (this
>> seems necessary in parse-fields-into-view) - any ideas on this?
>
> I don't think I get it. Do you mean a function that returns a
> presentation or a function the implements/acts as a presentation?
>

I meant a function that implements/acts as a presentation.  So
something like (lambda (value) (with-html (:input :value value :name
"test")) could just be dropped into the :present-as for a field (I
have some commented out code in presentation.lisp that would handle
this fine - it's more the parsing of lambda presentations that are
form fields that tripped me up).

>> 6) There is also a "view-alternate-version.lisp" which was a (i think
>> foolish) attempt at making a way for views to be mapped on top of
>> multiple models in a way that was different from just using mixed in
>> views.  I'm not sure if anyone sees any value to this (there is a
>> better explanation at the top of the file), but I've included it for
>> the curious.
>
> View composition -- does this make it possible to edit one large
> object in chunks, e.g. one slot per page?

I'm not sure if I understand you.  Would it not be possible to edit a
large object in chunks if the view for the object was created from a
bunch of mixed in views?  The widget still has full control over where
to render each field of the views and its mixed in views.  Or do you
mean something else?

Thanks!
Saikat

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"weblocks" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to 
[email protected]
For more options, visit this group at 
http://groups.google.com/group/weblocks?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to