Clojure

munge should not munge $ (which isJavaIdentifierPart), should munge ' (which is not)

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: Release 1.3
  • Component/s: None
  • Labels:
    None
  • Approval:
    Ok

Description

munge is overeager in converting $ to DOLLARSIGN, since $ is a valid
character in Java identifiers:

user> (filter #(Character/isJavaIdentifierPart %) (keys
clojure.lang.Compiler/CHAR_MAP))
(\$)

On a related point, ' (single quote) is not admissible in Java
identifiers, but it is not munged on master:

user=> (Character/isJavaIdentifierPart \')
false
user=> (munge "'")
"'"

This leads to e.g.

user=> *'
#<core$_STAR_' clojure.core$_STAR_'@5adf48c4>

(note the ' after STAR).

Originally reported on the Dev list.

See also this thread on the (regular) Clojure ggroup.

The attached patch applies cleanly against current master.

Activity

Hide
Michał Marczyk added a comment - - edited

Right, I forgot about that in the patch somehow. Also, I just noticed that " is also not JavaIdentifierPart and yet is not currently munged. The newly attached patch fixes all three issues.

Show
Michał Marczyk added a comment - - edited Right, I forgot about that in the patch somehow. Also, I just noticed that " is also not JavaIdentifierPart and yet is not currently munged. The newly attached patch fixes all three issues.
Hide
Rich Hickey added a comment -

This issue mentions two problems but patch fixes only one. Should add the single-quote handling

Show
Rich Hickey added a comment - This issue mentions two problems but patch fixes only one. Should add the single-quote handling
Hide
Stuart Halloway added a comment -

Patch is clean, the real issue here is doing the right thing. Two concerns:

  • was the original choice to munge $ motivated, and if so do we need a more subtle patch that preserves that original intent?
  • presumably this is (yet another) binary-compatibility-breaking change. 1.3 already has major change, so now is a good a time as any...
Show
Stuart Halloway added a comment - Patch is clean, the real issue here is doing the right thing. Two concerns:
  • was the original choice to munge $ motivated, and if so do we need a more subtle patch that preserves that original intent?
  • presumably this is (yet another) binary-compatibility-breaking change. 1.3 already has major change, so now is a good a time as any...
Show
Assembla Importer added a comment - Converted from http://www.assembla.com/spaces/clojure/tickets/433 Attachments: 0001-munge-no-longer-changes-to-DOLLARSIGN.patch - https://www.assembla.com/spaces/clojure/documents/dp5tziVaer34yIeJe5cbLr/download/dp5tziVaer34yIeJe5cbLr

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: