Clojure

Make LineNumberingPushbackReader's buffer size configurable

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: Release 1.2, Release 1.3, Release 1.4
  • Fix Version/s: Release 1.5
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code
  • Approval:
    Ok

Description

In some situations, it's necessary to configure the buffer size of LineNumberingPushbackReader's wrapped java.io.LineNumberReader, that gets created in the constructor. A concrete problem case is where you want to avoid doing reads from the underlying Reader whenever possible, so using a buffer size of 1 makes it a bit lazier. I can also imagine cases where you'd want to increase the buffer from java.io.BufferedReader's 8192-char default, but I haven't dealt with that one directly.

There's no good way to do this by subclassing LineNumberingPushbackReader, since all the action happens in the constructors: those of java.io.LineNumberReader and the superclass of LineNumberingPushbackReader, which is java.io.PushbackReader. So my current workaround is to copy the entirety of LineNumberingPushbackReader, change the name, and add a constructor. Having LineNumberingPushbackReader support this directly would be great.

Both java.io.LineNumberReader and java.io.PushbackReader have constructors that accept the buffer size as the second argument, so it seems very reasonable to me to add a similar constructor for LineNumberingPushbackReader.

Activity

Hide
Colin Jones added a comment - - edited

This patch still works great.

Turns out my current workaround to get around the lack of this ability has a problem: LispReader depends on the concrete LineNumberingPushbackReader class to be able to call .getLineNumber (via instanceof / casting). So the similar (read: nearly-copied) class I'm trying to use can't store line numbers, which makes the stack trace less nice (fwiw, this is in REPL-y: https://github.com/trptcolin/reply).

It would be nice to have an interface (ILineNumberingPushbackReader?) that declares getLineNumber, readLine, and atLineStart, and have things check instanceof on that instead of the concrete class. That way, anyone else adhering to the LineNumberingPushbackReader contract (in order to bind that to *in* as the docstring for `clojure.main/repl` prescribes) can do it and have line numbering play nicely with the Clojure reader.

If that sounds desirable, I can replace this patch. The existing patch will also work fine if we want to keep things concrete, or if that feels enough like solving a different problem to require another ticket.

Show
Colin Jones added a comment - - edited This patch still works great. Turns out my current workaround to get around the lack of this ability has a problem: LispReader depends on the concrete LineNumberingPushbackReader class to be able to call .getLineNumber (via instanceof / casting). So the similar (read: nearly-copied) class I'm trying to use can't store line numbers, which makes the stack trace less nice (fwiw, this is in REPL-y: https://github.com/trptcolin/reply). It would be nice to have an interface (ILineNumberingPushbackReader?) that declares getLineNumber, readLine, and atLineStart, and have things check instanceof on that instead of the concrete class. That way, anyone else adhering to the LineNumberingPushbackReader contract (in order to bind that to *in* as the docstring for `clojure.main/repl` prescribes) can do it and have line numbering play nicely with the Clojure reader. If that sounds desirable, I can replace this patch. The existing patch will also work fine if we want to keep things concrete, or if that feels enough like solving a different problem to require another ticket.
Hide
Fogus added a comment -

The patch looks fine.

Regarding the larger question of an interface I'd say another ticket is in order. The existence of LNPBR is an implementation detail, but I too have found it useful. Maybe there is a wider discussion on the usefulness of LNPBR and a sane way to expose it for tool, compiler, etc. devs?

Show
Fogus added a comment - The patch looks fine. Regarding the larger question of an interface I'd say another ticket is in order. The existence of LNPBR is an implementation detail, but I too have found it useful. Maybe there is a wider discussion on the usefulness of LNPBR and a sane way to expose it for tool, compiler, etc. devs?

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: