Clojure

Allow EdnReader to read foo// (CLJ-873 for EdnReader)

Details

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

Description

The patch that has been applied in 1.6 for CLJ-873 predated the introduction of EdnReader, as such it only patched LispReader.

This patch makes the same change to allow foo// in EdnReader, and removes two constants in clojure.lang.RT that are not needed anymore after this patch.

To reproduce:

user=> (require 'clojure.edn)
user=> (defn / [& x] 42)
user=> (clojure.edn/read-string "(user// 4 2)")
RuntimeException Invalid token: user//  clojure.lang.Util.runtimeException (Util.java:219)

Approach: copy changes from LispReader in CLJ-873. After:

user=> (require 'clojure.edn)
nil
user=> (defn / [& x] 42)
WARNING: / already refers to: #'clojure.core// in namespace: user, being replaced by: #'user//
#'user//
user=> (clojure.edn/read-string "(user// 4 2)")
(user// 4 2)

Patch: 0001-Fix-CLJ-873-for-EdnReader-too.patch

Screened by: Alex Miller

Activity

Hide
Alex Miller added a comment -

Switching back to prior patch.

Show
Alex Miller added a comment - Switching back to prior patch.
Hide
Alex Miller added a comment -

Andy you are uncanny - I was just on my way to update this.

Show
Alex Miller added a comment - Andy you are uncanny - I was just on my way to update this.
Hide
Andy Fingerhut added a comment -

With the undoing of the change for CLJ-1252 earlier today, the patch 0001-Fix-CLJ-873-for-EdnReader-too.patch applies cleanly again, and clj-1238-2.diff no longer does.

Show
Andy Fingerhut added a comment - With the undoing of the change for CLJ-1252 earlier today, the patch 0001-Fix-CLJ-873-for-EdnReader-too.patch applies cleanly again, and clj-1238-2.diff no longer does.
Hide
Alex Miller added a comment -

Updated patch to apply cleanly to master as of Oct 25, 2013.

Show
Alex Miller added a comment - Updated patch to apply cleanly to master as of Oct 25, 2013.
Hide
Andy Fingerhut added a comment -

I've not looked into details yet, but screened 0001-Fix-CLJ-873-for-EdnReader-too.patch fails to apply cleanly as of Oct 25, 2013 latest Clojure master.

Show
Andy Fingerhut added a comment - I've not looked into details yet, but screened 0001-Fix-CLJ-873-for-EdnReader-too.patch fails to apply cleanly as of Oct 25, 2013 latest Clojure master.
Hide
Nicola Mometto added a comment - - edited

Done - I also changed the description to better describe what the patch actually does

Show
Nicola Mometto added a comment - - edited Done - I also changed the description to better describe what the patch actually does
Hide
Alex Miller added a comment -

@Nicola - Please fix!

Show
Alex Miller added a comment - @Nicola - Please fix!
Hide
Nicola Mometto added a comment -

Alex, this title might be misleading – the main purpose of this patch is to allow "foo//" to be read by clojure.edn/read

Show
Nicola Mometto added a comment - Alex, this title might be misleading – the main purpose of this patch is to allow "foo//" to be read by clojure.edn/read

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: