aartaka
commented
1 year ago
attributes-default,attributes-keys-defaultand friends are poorly names, because they sound like they return the "default" of the attribute (which makes no sense), while it's the default attribute that's returned. Better names would bedefault-attribute, etc.
Agreed!
- Futures start being computed in
object-attributes. Can we defer to later? Can we add an option to choose when to compute it?
To allow even lazier evaluation and getting attribute values only when these are required? I don't see the immediate benefit, but that sounds good.
object-attributesshould return attribute objects instead of the ad-hoc lists.
Ad-hoc lists are so easy to work with 😞 But yeah, you're right here.
We've used list because of how attributes are declared in the
object-attributesmethods, but really there is no need to keep lists, we can very well use lists to declare opaqueattributeobjects, which then would have a proper API and which upstream would not destructure manually.
Yes!
- Considering the above, we need a better way to handle attribute options. Nyxt uses position 3 and 4 in the declaration for width calculation and styling. It should be stored in the new
attributeobject, but where? In a plist / hash-table? We need a way to refer to these options in a non-conflicting way: let's keep in mind that other libraries may have options with the same naming but different value. So we could index by package-prefixed symbols. Example(defmethod prompter:object-attributes ((buffer buffer) (source prompter:source)) (declare (ignore source)) `(("URL" ,(render-url (url buffer)) :html "..." :width 3) ("Title" ,(title buffer) :width 2) ("ID" ,(id buffer))))
If I got you right, you're suggesting returning a single attribute object from the prompter:object-attributes, with all the contents somehow indexed inside it. How about:
- Returning a list of
attributeobjects, - And then having
attributeto have avalueslot andnameslot, - With more slots added by the application-specific
attributesubclass.
This way we have both :width ... :html ... plists (initargs for a singular attribute), simple data model (lists of predictable attributes), and the lazy computation direly necessary for this API.
@aartaka What do you think? I remember you arguing in favour of positional options.
I don't remember why I did, but right now I'm on the side of having more structure 😅
Ambrevar
Follow up to #16. See https://github.com/atlas-engineer/prompter/pull/16#issuecomment-1582689618.
The attributes API suffers from bit rot.
What needs to be tweaked:
attributes-default,attributes-keys-defaultand friends are poorly named, because they sound like they return the "default" of the attribute (which makes no sense), while it's the default attribute that's returned. Better names would bedefault-attribute, etc.object-attributes. Can we defer to later? Can we add an option to choose when to compute it?object-attributesshould return attribute objects instead of the ad-hoc lists. We've used list because of how attributes are declared in theobject-attributesmethods, but really there is no need to keep lists, we can very well use lists to declare opaqueattributeobjects, which then would have a proper API and which upstream would not destructure manually.attributeobject, but where? In a plist / hash-table? We need a way to refer to these options in a non-conflicting way: let's keep in mind that other libraries may have options with the same naming but different value. So we could index by package-prefixed symbols. Examplewould return an
attributeobject with slotoptionsset to((nyxt:html "...") (nyxt:width 3)), or something like that.@aartaka What do you think? I remember you arguing in favour of positional options.