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:

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:

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

Patch: clj-1079-2.patch

Environment

None

Activity

Show:
Chas Emerick
October 7, 2012, 9:57 PM

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

Stuart Halloway
October 19, 2012, 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 Emerick
October 20, 2012, 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:

…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.

Andy Fingerhut
February 7, 2013, 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.

Andy Fingerhut
February 7, 2013, 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.

Completed

Assignee

Unassigned

Reporter

Chas Emerick

Labels

Approval

Ok

Patch

Code and Test

Fix versions

Affects versions

Priority

Major
Configure