Clojure

adds docstring support to defonce

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: Release 1.5
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test
  • Approval:
    Vetted

Description

Pass all args from defonce on to def so it supports docstrings (or potentially other future features) just like def.

Docstrings and other Var metadata will be lost when the defonce is reƫvaluated.

Patch: clj-1148-defonce-3.patch

Screened by: Stuart Sierra

  1. 0001-new-defonce-hotness.patch
    17/Jan/13 5:16 PM
    1 kB
    Joe Gallo
  2. clj-1148-defonce-2.patch
    29/Dec/13 10:24 PM
    3 kB
    Alex Miller
  3. clj-1148-defonce-3.patch
    10/Jan/14 5:04 PM
    3 kB
    Alex Miller
  4. defonce_fixes.patch
    24/Oct/13 12:44 PM
    3 kB
    Joe Gallo

Activity

Hide
Alex Miller added a comment -

Changed to defect for stomping metadata.

Show
Alex Miller added a comment - Changed to defect for stomping metadata.
Alex Miller made changes -
Field Original Value New Value
Issue Type Enhancement [ 4 ] Defect [ 1 ]
Alex Miller made changes -
Approval Triaged [ 10120 ]
Alex Miller made changes -
Labels docstring
Rich Hickey made changes -
Approval Triaged [ 10120 ] Vetted [ 10003 ]
Fix Version/s Release 1.6 [ 10157 ]
Stuart Halloway made changes -
Assignee Stuart Halloway [ stu ]
Hide
Stuart Halloway added a comment -

Please add tests. The clojure.test-helper namespace has useful temporary namespace support.

Show
Stuart Halloway added a comment - Please add tests. The clojure.test-helper namespace has useful temporary namespace support.
Stuart Halloway made changes -
Approval Vetted [ 10003 ] Incomplete [ 10006 ]
Hide
Joe Gallo added a comment -

This new patch includes the changes to defonce and also tests.

Show
Joe Gallo added a comment - This new patch includes the changes to defonce and also tests.
Joe Gallo made changes -
Attachment defonce_fixes.patch [ 12388 ]
Hide
Alex Miller added a comment -

Changing to Vetted so this is screenable again.

Show
Alex Miller added a comment - Changing to Vetted so this is screenable again.
Alex Miller made changes -
Approval Incomplete [ 10006 ] Vetted [ 10003 ]
Alex Miller made changes -
Description Two issues here:

1) defonce doesn't support docstrings, although def does, so it would be nice to bring the two in line with each other

2) defonce can stomp metadata, like this:

(defonce ^:private foo 5)
#'user/foo
(meta #'foo) --> the private is there
(defone foo 10)
foo is still 5
(meta #'foo) --> the private has been lost
Two issues here:

1) defonce doesn't support docstrings, although def does, so it would be nice to bring the two in line with each other

2) defonce can stomp metadata, like this:

(defonce ^:private foo 5)
#'user/foo
(meta #'foo) --> the private is there
(defone foo 10)
foo is still 5
(meta #'foo) --> the private has been lost

*Patch:* defonce_fixes.patch
*Screened by:*
Alex Miller made changes -
Approval Vetted [ 10003 ] Screened [ 10004 ]
Description Two issues here:

1) defonce doesn't support docstrings, although def does, so it would be nice to bring the two in line with each other

2) defonce can stomp metadata, like this:

(defonce ^:private foo 5)
#'user/foo
(meta #'foo) --> the private is there
(defone foo 10)
foo is still 5
(meta #'foo) --> the private has been lost

*Patch:* defonce_fixes.patch
*Screened by:*
Two issues here:

1) defonce doesn't support docstrings, although def does, so it would be nice to bring the two in line with each other

2) defonce can stomp metadata, like this:

(defonce ^:private foo 5)
#'user/foo
(meta #'foo) --> the private is there
(defone foo 10)
foo is still 5
(meta #'foo) --> the private has been lost

*Patch:* defonce_fixes.patch
*Screened by:* Alex Miller
Assignee Stuart Halloway [ stu ]
Rich Hickey made changes -
Issue Type Defect [ 1 ] Enhancement [ 4 ]
Hide
Rich Hickey added a comment -

I disagree about the stomp metadata - different metadata was provided. The purpose of defonce is to avoid the re-evaluation of the init. Is this the simplest change that accomplishes the doc string? In any case split in two.

Show
Rich Hickey added a comment - I disagree about the stomp metadata - different metadata was provided. The purpose of defonce is to avoid the re-evaluation of the init. Is this the simplest change that accomplishes the doc string? In any case split in two.
Rich Hickey made changes -
Approval Screened [ 10004 ] Incomplete [ 10006 ]
Hide
Alex Miller added a comment -

Reduced scope of ticket to just passing defonce args on to def to add support for docstring. Added new patch that does this.

Show
Alex Miller added a comment - Reduced scope of ticket to just passing defonce args on to def to add support for docstring. Added new patch that does this.
Alex Miller made changes -
Patch Code [ 10001 ] Code and Test [ 10002 ]
Approval Incomplete [ 10006 ] Vetted [ 10003 ]
Description Two issues here:

1) defonce doesn't support docstrings, although def does, so it would be nice to bring the two in line with each other

2) defonce can stomp metadata, like this:

(defonce ^:private foo 5)
#'user/foo
(meta #'foo) --> the private is there
(defone foo 10)
foo is still 5
(meta #'foo) --> the private has been lost

*Patch:* defonce_fixes.patch
*Screened by:* Alex Miller
Pass all args from {{defonce}} on to {{def}} so it supports docstrings (or potentially other future features) just like def.

*Patch:* clj-1148-defonce-2.patch
*Screened by:*
Attachment clj-1148-defonce-2.patch [ 12569 ]
Summary adds docstring support to defonce, and stops it from stomping existing metadata adds docstring support to defonce
Hide
Stuart Sierra added a comment -

Screened clj-1148-defonce-2.patch but returning to 'incomplete' status.

The :arglists metadata in this patch (a list of symbols) is inconsistent with all other uses of :arglists (a list of vectors).

Other than that the patch is good.

Show
Stuart Sierra added a comment - Screened clj-1148-defonce-2.patch but returning to 'incomplete' status. The :arglists metadata in this patch (a list of symbols) is inconsistent with all other uses of :arglists (a list of vectors). Other than that the patch is good.
Stuart Sierra made changes -
Approval Vetted [ 10003 ] Incomplete [ 10006 ]
Hide
Alex Miller added a comment -

Updated patch to address inconsistency in arglist format and attached clj-1148-defonce-3.patch.

Show
Alex Miller added a comment - Updated patch to address inconsistency in arglist format and attached clj-1148-defonce-3.patch.
Alex Miller made changes -
Approval Incomplete [ 10006 ] Vetted [ 10003 ]
Description Pass all args from {{defonce}} on to {{def}} so it supports docstrings (or potentially other future features) just like def.

*Patch:* clj-1148-defonce-2.patch
*Screened by:*
Pass all args from {{defonce}} on to {{def}} so it supports docstrings (or potentially other future features) just like def.

*Patch:* clj-1148-defonce-3.patch
*Screened by:*
Attachment clj-1148-defonce-3.patch [ 12692 ]
Hide
Stuart Sierra added a comment -

The patch clj-1148-defonce-3.patch is OK but it doesn't really address the docstring issue because defonce still destroys metadata. For example:

user=> (defonce foo "docstring for foo" (do (prn 42) 42))
42
#'user/foo
user=> (doc foo)
-------------------------
user/foo
  docstring for foo
nil
user=> (defonce foo "docstring for foo" (do (prn 42) 42))
nil
user=> (doc foo)
-------------------------
user/foo
  nil
Show
Stuart Sierra added a comment - The patch clj-1148-defonce-3.patch is OK but it doesn't really address the docstring issue because defonce still destroys metadata. For example:
user=> (defonce foo "docstring for foo" (do (prn 42) 42))
42
#'user/foo
user=> (doc foo)
-------------------------
user/foo
  docstring for foo
nil
user=> (defonce foo "docstring for foo" (do (prn 42) 42))
nil
user=> (doc foo)
-------------------------
user/foo
  nil
Hide
Stuart Sierra added a comment -

Screened with reservations noted.

Show
Stuart Sierra added a comment - Screened with reservations noted.
Stuart Sierra made changes -
Approval Vetted [ 10003 ] Screened [ 10004 ]
Description Pass all args from {{defonce}} on to {{def}} so it supports docstrings (or potentially other future features) just like def.

*Patch:* clj-1148-defonce-3.patch
*Screened by:*
Pass all args from {{defonce}} on to {{def}} so it supports docstrings (or potentially other future features) just like def.

Docstrings and other Var metadata will be lost when the {{defonce}} is reëvaluated.

*Patch:* clj-1148-defonce-3.patch

*Screened by:* Stuart Sierra
Hide
Rich Hickey added a comment -

Stuart is right, second defonce should retain the doc string (since it again provides it, should be no-op)

Show
Rich Hickey added a comment - Stuart is right, second defonce should retain the doc string (since it again provides it, should be no-op)
Rich Hickey made changes -
Approval Screened [ 10004 ] Incomplete [ 10006 ]
Hide
Alex Miller added a comment -

pull out of 1.6

Show
Alex Miller added a comment - pull out of 1.6
Alex Miller made changes -
Approval Incomplete [ 10006 ] Vetted [ 10003 ]
Fix Version/s Release 1.6 [ 10157 ]

People

Vote (4)
Watch (2)

Dates

  • Created:
    Updated: