Make LineNumberingPushbackReader's buffer size configurable

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.

Environment

None

Attachments

1

Activity

Show:

Fogus August 15, 2012 at 7:49 PM

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?

Colin Jones January 23, 2012 at 12:56 AM

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.

Completed

Details

Assignee

Reporter

Approval

Ok

Patch

Code

Priority

Fix versions

Created January 12, 2012 at 1:16 AM
Updated August 18, 2012 at 1:50 PM
Resolved August 18, 2012 at 1:50 PM