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. clj-1148-defonce-4.patch
    28/Aug/14 12:08 PM
    3 kB
    Linus Ericsson
  5. clj-1148-defonce-4.patch
    28/Aug/14 12:07 PM
    3 kB
    Linus Ericsson
  6. clj-1148-defonce-5.patch
    28/Aug/14 12:30 PM
    3 kB
    Linus Ericsson
  7. clj-1148-defonce-6.patch
    09/Sep/14 4:27 AM
    1 kB
    Linus Ericsson
  8. defonce_fixes.patch
    24/Oct/13 12:44 PM
    3 kB
    Joe Gallo

Activity

Hide
Linus Ericsson added a comment -

Yet another, simpler version of defonce. No test-cases included.

This version just makes an (or (nil? v#) (not (.hasRoot v#)) test on the resolved variable. If this is true, really define by (def ~name ~@args) else do nothing.

Show
Linus Ericsson added a comment - Yet another, simpler version of defonce. No test-cases included. This version just makes an (or (nil? v#) (not (.hasRoot v#)) test on the resolved variable. If this is true, really define by (def ~name ~@args) else do nothing.
Hide
Linus Ericsson added a comment - - edited

This version looks for previously defined var with resolve. A repeated defonce won't affect the namespace at all if the variable is already defined and bounded.

Please confirm using (resolve '~name) is not a problem w.r.t ns-bindings or similar.

This patch also contains the tests from clj-1148-defonce-3.patch as well as the :arglists property.

(patch 4 missed one def-row, sorry for mailbox noise).

Show
Linus Ericsson added a comment - - edited This version looks for previously defined var with resolve. A repeated defonce won't affect the namespace at all if the variable is already defined and bounded. Please confirm using (resolve '~name) is not a problem w.r.t ns-bindings or similar. This patch also contains the tests from clj-1148-defonce-3.patch as well as the :arglists property. (patch 4 missed one def-row, sorry for mailbox noise).
Hide
Alex Miller added a comment -

pull out of 1.6

Show
Alex Miller added a comment - pull out of 1.6
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)
Hide
Stuart Sierra added a comment -

Screened with reservations noted.

Show
Stuart Sierra added a comment - Screened with reservations noted.
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
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.
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.
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.
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.
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.
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.
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.
Hide
Alex Miller added a comment -

Changed to defect for stomping metadata.

Show
Alex Miller added a comment - Changed to defect for stomping metadata.

People

Vote (5)
Watch (3)

Dates

  • Created:
    Updated: