Prototype of a new view system
Saikat Chakrabarti
new view system could look and committed it to my branch (http://
bitbucket.org/saikat/weblocks-saikat/overview/). The new view/
presentation code is in contrib/saikat , and I've included a short
example that uses them in examples/view-refactor-demo . A few notes
about what you'll find there:
1) I mostly want to get feedback on the structure of things/generally
how everything fits together (and comments on implementation of the
big stuff is good too). The general idea is to scale back the
responsibility of views to just be maps between models and widgets,
and have widgets be responsible for rendering. Views themselves hold
fields, where each field is a widget that has a corresponding
presentation. So when I actually refactor the code in weblocks, a lot
of the view rendering stuff will be put into the rendering for the
form widget (as is evident some from my example).
2) I haven't yet fleshed out the syntax for actually defining views
(right now, as you'll see, it's just using defclass and is clunky). I
think eventually I want something fairly similar to how it used to be
(like (defview view ((field-1 :parse-as input :reader (lambda
(obj)...)))), and have the things in the defview be defaults that are
used per instance (but overwriteable by each instance). Any thoughts
on this?
3) One thing I wasn't sure about was whether views should keep state
or not. I ultimately went with stateful view fields because I like
the idea of having the fields be pure widgets that can be marked dirty
(it lets me do things like what I do in the example of just redrawing
certain fields without redrawing the whole containing widget). Also,
views seem to me to be a layer on top of models the same way models
are a layer over the database, so it seems natural to me to interact
with them similarly (set values and then persist when ready).
4) Much of this code is not well tested/buggy, and there is a lot of
stuff missing that the old views have (that I will add in later - for
example, validation just returns true right now, but the stub I have
is where validation will go) - I mostly just wanted to show off the
diff.
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?
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.
The big differences:
1) Views are not singletons
2) Views have state
3) View fields are widgets
Any feedback will be much appreciated!
-Saikat
Leslie P. Polzer
I agree with your design decisions so far, especially with the
statefulness of fields.
Some first suggestions:
Please try to split up the code into multiple files, preferably
one per class. It's cumbersome to review these huge chunks of code.
(defmethod initialize-instance :around ((obj stateful-view) &key)
(let ((*current-view* obj))
(when (next-method-p)
(call-next-method))))
I didn't look it up in the spec but I guess we can rely on there
being a default initialize-instance so the check for the next method
is moot.
Why is it necessary to supply this variable to the next method(s)?
Same for *current-presentation*.
(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.
(validator
:initform nil
:initarg :validator
:accessor stateful-field-validator
Don't forget that it should be possible to specify multiple
validators.
(data-class
:initform nil
:initarg :data
:accessor stateful-view-data-class
Why not always derive the data class store via CLASS-STORE?
(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.
(defun stringified-slot-names (obj)
Is this really necessary?
;; 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?
> 2) I haven't yet fleshed out the syntax for actually defining views
> (right now, as you'll see, it's just using defclass and is clunky). I
> think eventually I want something fairly similar to how it used to be
> (like (defview view ((field-1 :parse-as input :reader (lambda
> (obj)...)))), and have the things in the defview be defaults that are
> used per instance (but overwriteable by each instance). Any thoughts
> on this?
This sounds like a good way to do it. I don't think it will require
much work to adapt the current DSL compiler to it, either.
> 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?
> 4) Much of this code is not well tested/buggy, and there is a lot of
> stuff missing that the old views have (that I will add in later - for
> example, validation just returns true right now, but the stub I have
> is where validation will go) - I mostly just wanted to show off the
> diff.
Yes, very good!
> 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?
> 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?
Thanks so far, I think we're on the right way!
Leslie
Saikat Chakrabarti
> 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
Leslie P. Polzer
> > (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.
We can keep it like this and see whether it's enough.
> > (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?
We should have validation both at the view and field level, and for both
it should be possible to specify multiple validators.
This is the current implementation.
> > (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).
Well, yes -- I guess
#'object-store =~ (compose #'class-store #'class-of)
> 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.
The dataform needs to have a data class because it supports creating
new objects if it is not given any.
So this is a special requirement of dataform, not of views in general.
There's no need to assume that. If you like to keep your models in a
separate package you just need to export the view field name symbols
from it and make the ui package use it.
> >> 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).
Why not just make the compiler take the lambda and use it to create
a FUNCALL-PRESENTATION?
> 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?
I think my question is moot considering that view fields can hold their
own data now and thus are decoupled from the underlying data object.
Leslie