Skip to:

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

Readers need [LineNumbering]PushbackReader, but no helper exists to make these

Description

The reader functions (clojure.core/read and clojure.edn/read) require a PushbackReader but all of the clojure.java.io helpers will make a BufferedReader, which is insufficient. Thus, any use of the read functions tends to look like:

(require '[clojure.edn :as edn] '[clojure.java.io :as jio]) (import java.io.PushbackReader) (with-open [r (PushbackReader. (jio/reader "config.edn"))] (read r)])

Additionally, the newer clojure.core/read+string requires a LineNumberingPushbackReader which is a Clojure implementation class. In practice, many uses of the reader also do use a LNPR even though they only require a PBR.

This disconnect between the reader inputs and what clojure.java.io provides is a common source of confusion/frustration for developers as they need something in between them to plug them together, and (particularly with LNPR) it’s not obvious where to get that.

Alternatives

  1. clojure.java.io reader function could return a PBR

  2. read functions could relax inputs to just Reader and coerce inputs as needed to PBRs - has been rejected in the past

  3. add helper functions in clojure.java.io to produce PBR and LNPBR

Can’t do #1 because we currently promise a BufferedReader and BR and PBR are both extension classes that add their own methods to the base Reader. Other functions in core (and in the world) already rely on BR methods like .readLine.

Can’t do #2 because PBR is stateful. If you are going to call read multiple times, you need to retain any potential pushed back state in the PBR for subsequent reads. If you wrap inside read, there is no way to return the stateful reader too (given the existing interface which returns the value which may not necessarily be able to carry metadata).

#3 is purely additive so seems like a valuable addition (tools.reader went this direction long ago).

Questions

There are really two types here - java.io.PushbackReader and clojure.lang.LineNumberingPushbackReader. PBR wraps any Reader and has just the pushback buffer (default size = 1, which is all the LispReader needs). LNPR extends PBR but wraps a java.io.LineNumberReader, which is a subclass of BufferedReader, which uses an 8k buffer by default. Both of these options are used in Clojure itself in different contexts.

  1. Should we have one new function pushback-reader with a :line-number option or a second function?

  2. The existing reader function always returns a BufferedReader and the existing LineNumberingPushbackReader constructors will wrap incoming Readers into a BufferedReader subclass. PushbackReader constructors do not. We could:

    1. Have both pushback-reader and the line numbered version call the reader function, but I think you’d have an unnecessary level of buffered reader in the line numbered case.

    2. Have neither wrap the reader function, but you could easily combine reader and pushback-reader to get that

    3. Be more judicious about whether we wrap? (if (instance? BufferedReader x) x (reader ...))

Environment

None

Attachments

2
  • 11 Jan 2015, 05:14 PM
  • 10 Jan 2015, 10:05 PM

Activity

Show:

Alex Miller January 13, 2023 at 4:12 PM

Thanks for the bump - would be good if you upvote at https://ask.clojure.org/index.php/1957/clojure-java-io-pushback-reader too

Kyle Kingsbury January 13, 2023 at 3:26 PM

Just registering my support for merging this. Every time I go to read EDN from a file--roughly once a month--I bump my head on the need for a PushbackReader, have to go look up what package it’s in, import it, wrap the reader, etc. It’d be really nice if this were just a part of clojure.java.io--it’s a small change and you have to do this dance basically every time you want to work with EDN.

David Rupp January 11, 2015 at 5:14 PM

Adding drupp-clj-1611-2.patch to address previous comments.

import January 11, 2015 at 1:54 PM

Comment made by: pbwolf

clojure.java.io/reader is idempotent, while the patch of 10-Jan-2015 re-wraps an existing PushbackReader twice: first with a new BufferedReader, then with a new PushbackReader.

Leaving a given PushbackReader alone would be more in keeping with the pattern of clojure.java.io.

It also needs a docstring. If pushback-reader were idempotent, the docstring's opening phrase could echo clojure.java.io/reader's, e.g.: Attempts to coerce its argument to java.io.PushbackReader; failing that, (bla bla bla).

David Rupp January 10, 2015 at 10:07 PM

Note that you can always import java.io.PushbackReader and do something like (PushbackReader. (reader my-thing)) yourself; that's really all the patch does.

Details

Assignee

Reporter

Labels

Approval

Vetted

Priority

Affects versions

Created December 9, 2014 at 1:10 AM
Updated September 5, 2023 at 1:59 PM

Flag notifications