Clojure

print/read syntax for defrecords

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: Release 1.3
  • Fix Version/s: Release 1.3
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test
  • Approval:
    Ok

Activity

Hide
Assembla Importer added a comment -
Show
Assembla Importer added a comment - Converted from http://www.assembla.com/spaces/clojure/tickets/374
Hide
Assembla Importer added a comment -

stu said: If we are going to use #=, then how about modify record to take an optional metadata map before the keyvals:

(defn record
  "Construct an instance of record-type from keyvals.

   record-type is a keyword naming the record.
   keyvals can include *any* keyword/value pairs, and
   can be preceded by an optional metadata map."
  {:added "1.2"}
  [record-type & keyvals]
  {:pre [(keyword? record-type)]}
  (let [meta (when (map? (first keyvals)) (first keyvals))
        keyvals (if meta (rest keyvals) keyvals)]
    (record-implementation-detail-multimethod record-type meta (apply hash-map keyvals))))
</code></pre>

and then the read syntax can be

<pre><code>
#=(record :user.Foo {:some-meta 1} :a nil :c 3)
Show
Assembla Importer added a comment - stu said: If we are going to use #=, then how about modify record to take an optional metadata map before the keyvals:
(defn record
  "Construct an instance of record-type from keyvals.

   record-type is a keyword naming the record.
   keyvals can include *any* keyword/value pairs, and
   can be preceded by an optional metadata map."
  {:added "1.2"}
  [record-type & keyvals]
  {:pre [(keyword? record-type)]}
  (let [meta (when (map? (first keyvals)) (first keyvals))
        keyvals (if meta (rest keyvals) keyvals)]
    (record-implementation-detail-multimethod record-type meta (apply hash-map keyvals))))
</code></pre>

and then the read syntax can be

<pre><code>
#=(record :user.Foo {:some-meta 1} :a nil :c 3)
Hide
Assembla Importer added a comment -

richhickey said: I think you need to distinguish between print and print-dup contexts. I don't like the switch-on-type-of-first-arg in general. We don't do optional args preceding variadics. Also, normally we don't see metatadata in print unless print-meta is true.

I think a #= ctor call works for print-dup:

#=(Foo. 1 2 3 {:my :meta} {:other :field})

eliding the meta and extmap when neither is present:

#(Foo. 1 2 3)

not sure about ordinary print yet

Show
Assembla Importer added a comment - richhickey said: I think you need to distinguish between print and print-dup contexts. I don't like the switch-on-type-of-first-arg in general. We don't do optional args preceding variadics. Also, normally we don't see metatadata in print unless print-meta is true. I think a #= ctor call works for print-dup: #=(Foo. 1 2 3 {:my :meta} {:other :field}) eliding the meta and extmap when neither is present: #(Foo. 1 2 3) not sure about ordinary print yet
Hide
Assembla Importer added a comment -

stu said: In the above, why isn't print-dup's effect recursive, e.g.

#=(user.Foo 1 2 3 #=(clojure.lang.PersistentArrayMap/create {:my :meta}) 
                                 #=(clojure.lang.PersistentArrayMap/create {:other :field}))

print-dup on maps certainly behaves that way, including nested maps.

Also, isn't a print-dup version that includes metadata only correct when print-meta is false?

Show
Assembla Importer added a comment - stu said: In the above, why isn't print-dup's effect recursive, e.g.
#=(user.Foo 1 2 3 #=(clojure.lang.PersistentArrayMap/create {:my :meta}) 
                                 #=(clojure.lang.PersistentArrayMap/create {:other :field}))
print-dup on maps certainly behaves that way, including nested maps. Also, isn't a print-dup version that includes metadata only correct when print-meta is false?
Hide
Assembla Importer added a comment -

stu said: Updating tickets (#370, #366, #374)

Show
Assembla Importer added a comment - stu said: Updating tickets (#370, #366, #374)
Stuart Halloway made changes -
Field Original Value New Value
Assignee Stuart Halloway [ stu ]
Aaron Bedra made changes -
Reporter Assembla Importer [ importer ]
Priority Major [ 3 ]
Christopher Redinger made changes -
Assignee Fogus [ fogus ]
Christopher Redinger made changes -
Approval Vetted
Christopher Redinger made changes -
Waiting On stu
Hide
Fogus added a comment -

Patch scoped to the discussion in this ticket and at http://dev.clojure.org/display/design/defrecord+improvements

Show
Fogus added a comment - Patch scoped to the discussion in this ticket and at http://dev.clojure.org/display/design/defrecord+improvements
Fogus made changes -
Attachment CLJ-374-printread-syntax-for-defrecords.patch [ 10206 ]
Approval Vetted Test
Hide
Fogus added a comment -

Added IRecord patch file. Sorry.

Show
Fogus added a comment - Added IRecord patch file. Sorry.
Fogus made changes -
Attachment CLJ-374-printread-syntax-for-defrecords-irecord.patch [ 10207 ]
Stuart Halloway made changes -
Comment [ should the vector returned by getBasis have type hints, if they were provided when creating the record? ]
Hide
Stuart Halloway added a comment -

Using primitive type hints seems to break the map->Record constructor:

(defrecord Bar [^long a b])
(map->Bar {:a 1 :b 2})
=> NullPointerException   clojure.lang.RT.longCast (RT.java:1008)
Show
Stuart Halloway added a comment - Using primitive type hints seems to break the map->Record constructor:
(defrecord Bar [^long a b])
(map->Bar {:a 1 :b 2})
=> NullPointerException   clojure.lang.RT.longCast (RT.java:1008)
Hide
Stuart Halloway added a comment -

The patch does not address deftype. It gets (as a freebie) all the new stuff available to arbitrary Java classes, but no create method, positional fn, etc. I think we should do deftype as a separate patch.

Show
Stuart Halloway added a comment - The patch does not address deftype. It gets (as a freebie) all the new stuff available to arbitrary Java classes, but no create method, positional fn, etc. I think we should do deftype as a separate patch.
Hide
Fogus added a comment -

RE: longCast

I will check that out. Thanks

RE: deftype

deftype does get a create and getBasis method, but the create is breaking because I am assuming a record ctor. I can fix that if we think that types should have a create that takes a map.

(deftype T [a])
(user.T/getBasis)
;=> [a]

(user.T/create {:a 1})
;; NoSuchMethodError

This is because I'm building a call to the record ctor that takes metadata and an extension map. Does it make sense to generate such a create that takes a map for types?

Thanks
:F

Show
Fogus added a comment - RE: longCast I will check that out. Thanks RE: deftype deftype does get a create and getBasis method, but the create is breaking because I am assuming a record ctor. I can fix that if we think that types should have a create that takes a map. (deftype T [a]) (user.T/getBasis) ;=> [a] (user.T/create {:a 1}) ;; NoSuchMethodError This is because I'm building a call to the record ctor that takes metadata and an extension map. Does it make sense to generate such a create that takes a map for types? Thanks :F
Fogus made changes -
Attachment CLJ-374-printread-syntax-for-defrecords-irecord.patch [ 10207 ]
Fogus made changes -
Attachment CLJ-374-printread-syntax-for-defrecords.patch [ 10206 ]
Hide
Fogus added a comment -

minor fixes from last patch set.

Show
Fogus added a comment - minor fixes from last patch set.
Fogus made changes -
Attachment CLJ-374-defrecord-literals.patch [ 10216 ]
Hide
Rich Hickey added a comment -

deftypes shouldn't get any map-taking support as they are not mappy (mappish?). They should get positional support. defrecord and deftype stuff should go in together.

Show
Rich Hickey added a comment - deftypes shouldn't get any map-taking support as they are not mappy (mappish?). They should get positional support. defrecord and deftype stuff should go in together.
Fogus made changes -
Attachment CLJ-374-defrecord-literals.patch [ 10216 ]
Hide
Fogus added a comment -

Latest patch includes changes to support deftype literals and removal of RecordExpr.

Show
Fogus added a comment - Latest patch includes changes to support deftype literals and removal of RecordExpr.
Fogus made changes -
Attachment CLJ-374-defrecord-literals.patch [ 10221 ]
Christopher Redinger made changes -
Attachment 1.3-beta-release-notes.diff [ 10227 ]
Christopher Redinger made changes -
Attachment 1.3-beta-release-notes.diff [ 10227 ]
Christopher Redinger made changes -
Comment [ With the alpha 7 stuff included ]
Hide
Christopher Redinger added a comment - - edited

This patch has already been applied. Updating status of this ticket.

https://github.com/clojure/clojure/commit/ac1e8ad9f182dc2e8a5254f3e4b7b77c0258353d

Show
Christopher Redinger added a comment - - edited This patch has already been applied. Updating status of this ticket. https://github.com/clojure/clojure/commit/ac1e8ad9f182dc2e8a5254f3e4b7b77c0258353d
Christopher Redinger made changes -
Affects Version/s Release.Next [ 10038 ]
Approval Test Screened
Patch Code and Test
Rich Hickey made changes -
Approval Screened Ok
Stuart Halloway made changes -
Status In Progress [ 3 ] Closed [ 6 ]
Resolution Completed [ 1 ]

People

Vote (1)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: