Skip to:

  1. Skip to Jira Navigation
  2. Skip to Side Navigation
  3. Skip to Main Content

In reader, don't ignore explicit :line :column meta

Description

Unfortunately, when read in using a LineNumberingPushbackReader, code like this has its :line metadata squashed by the line numbers coming from that. A REPL-friendly example would be:

=> (meta (read (clojure.lang.LineNumberingPushbackReader. (java.io.StringReader. "^{:line 66} ()")))) {:line 1, :column 1} => (meta (read (java.io.PushbackReader. (java.io.StringReader. "^{:line 66} ()")))) {:line 66, :column 1}

The latter seems more correct to me (and is equivalent to read-string).

Proposed: Retain existing :line/:column meta on seqs and lists that are read with meta rather than overriding with the values read from the reader.

Additionally, use :file meta as a standard way for the REPL to allow external tool using the repl to specify the source context:

user=> ^{:file "hi.clj" :line 100 :column 10} (def 5) Syntax error compiling def at (hi.clj:100:10). First argument to def must be a Symbol

The repl is finding :file meta and binding SOURCE_PATH if it sees it.

Patch: clj-1079-2.patch

Environment

None

Attachments

2
  • 12 Oct 2018, 07:51 PM
  • 07 Oct 2012, 09:57 PM

Activity

Show:

Andy FingerhutFebruary 7, 2013 at 6:35 PM

Sorry for the noise. I've removed clj-1079-patch-v2.txt mentioned in the previous comment, because I learned that CLJ-1079.diff dated Oct 7 2012 applies cleanly to latest master and passes all tests if you use this command to apply it.

% git am --keep-cr -s --ignore-whitespace < CLJ-1079.diff

I will update the JIRA workflow page instructions for applying patches to mention this, too, because there are multiple patches that fail without --ignore-whitespace, but apply cleanly with that option. That will eliminate the need to update patches merely for whitespace changes.

Andy FingerhutFebruary 7, 2013 at 2:47 PM

clj-1079-patch-v2.txt dated Feb 7 2013 is identical to Chas's CLJ-1079.diff dated Oct 7 2012, except it applies cleanly to latest master. I believe the only difference is that some white space in the context lines is updated.

Chas EmerickOctober 20, 2012 at 1:36 AM

The information being conveyed is the same :line and :column metadata conveyed by LispReader — in fact, that's where it comes from in the first place.

Kibit (and cljx) is essentially an out-of-band source transformation tool. Given an input like this:

(ns com.foo.hosty) (defn ^:clj system-hash [x] (System/identityHashCode x)) (defn ^:cljs system-hash [x] (goog/getUid x))

…it produces two files, a .clj for Clojure, and a .cljs for ClojureScript. (The first code listing in the ticket description is the former.)

However, because there's no way to transform Clojure code/data without losing formatting, anything that depends on line/column numbers (stack traces, stepping debuggers) is significantly degraded. If LispReader were to defer to :line and :column metadata already available on the loaded forms (there when the two generated files are spit out with print-meta on), this would not be the case.

Stuart HallowayOctober 19, 2012 at 9:12 PM

This feels backwards to me. If a special purpose tool wants to convey information via metadata, why does it use names that collide with those used by LispReader?

Chas EmerickOctober 7, 2012 at 9:57 PM

Refreshed patch to apply cleanly to master after the recent off by one patch for :column metadata.

Completed

Details

Assignee

Reporter

Labels

Approval

Ok

Patch

Code and Test

Priority

Affects versions

Fix versions

Created September 29, 2012 at 11:12 PM
Updated November 6, 2018 at 12:33 PM
Resolved November 6, 2018 at 12:33 PM

Flag notifications