Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address #166: Set history file max size from inputrc via JLine #183

Merged
merged 2 commits into from Jul 19, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,21 @@
# REPLy Changelog

## Unreleased
- #166: Set history file max size from inputrc
- Use newer jline2 to support history max size config

## 0.4.0

## 0.3.10

## 0.3.8

## 0.3.7

## 0.3.6

## 0.3.5

## 0.3.4 (there was no 0.3.3), 2014-08-06
- Exclude nrepl from drawbridge dependency

Expand Down
2 changes: 1 addition & 1 deletion project.clj
Expand Up @@ -4,7 +4,7 @@
(defproject reply "0.4.1-SNAPSHOT"
:description "REPL-y: A fitter, happier, more productive REPL for Clojure."
:dependencies [[org.clojure/clojure "1.6.0"]
[jline "2.14.5"]
[jline "2.14.6"]
[org.thnetos/cd-client "0.3.6"]
[clj-stacktrace "0.2.7"]
[nrepl "0.4.1"]
Expand Down
12 changes: 12 additions & 0 deletions spec/reply/integration_spec.clj
Expand Up @@ -58,6 +58,7 @@
(java.io.ByteArrayInputStream.
(.getBytes "(* 21 2)\n"))
:output-stream @fake-out})
(should= "" (str @fake-err))
(should-contain "42" (str @fake-out))
(should-contain (with-out-str (initialization/help)) (str @fake-out))
(should-contain "Bye for now!\n" (str @fake-out)))
Expand All @@ -67,27 +68,31 @@
(java.io.ByteArrayInputStream.
(.getBytes "\"test\"\n"))
:output-stream @fake-out})
(should= "" (str @fake-err))
(should-contain "\"test\"" (str @fake-out)))

(it "prints an error when given something that can't be read"
(main/launch-standalone {:input-stream
(java.io.ByteArrayInputStream.
(.getBytes ")\n"))
:output-stream @fake-out})
(should= "" (str @fake-err))
(should-contain "Unmatched delimiter" (str @fake-out)))

(it "puts read-time errors into *e"
(main/launch-standalone {:input-stream
(java.io.ByteArrayInputStream.
(.getBytes ")\n*e\n"))
:output-stream @fake-out})
(should= "" (str @fake-err))
(should-contain #"Unmatched delimiter.+\n.+\n.+Unmatched delimiter" (str @fake-out)))

(it "does not print an error when given empty input lines"
(main/launch-standalone {:input-stream
(java.io.ByteArrayInputStream.
(.getBytes "\n\n\n"))
:output-stream @fake-out})
(should= "" (str @fake-err))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this addition actually obviates these existing should-not-contain checks on stdout, or if those errors might really appear on stdout if a bug is present.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm, great question - I suspect you're right that these should've been checks against fake-err. In any case, non-blocker. Thanks!

(should-not-contain "EOF while reading" (str @fake-out))
(should-not-contain "RuntimeException" (str @fake-out)))

Expand All @@ -96,13 +101,15 @@
(java.io.ByteArrayInputStream.
(.getBytes "424242\n\n\n(* 2 *1)\n"))
:output-stream @fake-out})
(should= "" (str @fake-err))
(should-contain "848484" (str @fake-out)))

(it "tab-completes clojure.core fns"
(main/launch-standalone {:input-stream
(java.io.ByteArrayInputStream.
(.getBytes "map\t\nexit\n"))
:output-stream @fake-out})
(should= "" (str @fake-err))
(should-contain "mapcat" (str @fake-out))
(should-contain "map-indexed" (str @fake-out)))

Expand All @@ -111,13 +118,15 @@
(java.io.ByteArrayInputStream.
(.getBytes "424242\nmap\t\b\b\b(* 2 *1)\n"))
:output-stream @fake-out})
(should= "" (str @fake-err))
(should-contain "848484" (str @fake-out)))

(it "does not crash when printing throws an exception"
(main/launch-standalone {:input-stream
(java.io.ByteArrayInputStream.
(.getBytes "(lazy-seq #())\n"))
:output-stream @fake-out})
(should= "" (str @fake-err))
; If we're not crashing, output should lack a full stack trace
; that starts with clojure.lang.LazySeq.sval
(should-not-contain "LazySeq" (str @fake-out)))
Expand Down Expand Up @@ -166,6 +175,7 @@
(java.io.ByteArrayInputStream.
(.getBytes "exit\n(println 'foobar)\n"))
:output-stream @fake-out})
(should= "" (str @fake-err))
(should-contain (with-out-str (initialization/help)) (str @fake-out))
(should-not-contain "foobar" (str @fake-out))
(should-contain "Bye for now!\n" (str @fake-out)))
Expand All @@ -176,6 +186,7 @@
(java.io.ByteArrayInputStream.
(.getBytes "(doc map)\nexit\n"))
:output-stream @fake-out})
(should= "" (str @fake-err))
(should-contain (with-out-str (clojure.repl/doc map))
(str @fake-out))))

Expand All @@ -187,5 +198,6 @@
(java.io.ByteArrayInputStream.
(.getBytes "map\t\nexit\n"))
:output-stream @fake-out})
(should= "" (str @fake-err))
(should-contain "mapcat" (str @fake-out))
(should-contain "map-indexed" (str @fake-out)))))
30 changes: 27 additions & 3 deletions src/clj/reply/reader/simple_jline.clj
Expand Up @@ -2,7 +2,7 @@
(:require [reply.reader.jline.completion :as jline.completion])
(:import [java.io File FileInputStream FileDescriptor
PrintStream ByteArrayOutputStream IOException]
[jline.console ConsoleReader]
[jline.console ConsoleReader ConsoleKeys]
[jline.console.history FileHistory MemoryHistory]
[jline.internal Configuration Log]))

Expand Down Expand Up @@ -68,6 +68,26 @@
(when-let [completer (first (.getCompleters reader))]
(.removeCompleter reader completer))))

(defn ^Integer default-history-size
"Get default history size from system settings, or nil if not set.
Set `app-name` for jline's reading of inputrc, or null for default."
[^String app-name]
(try
(some-> (ConsoleKeys. app-name (ConsoleReader/getInputRc))
(.getVariable "history-size")
(Integer/parseInt))
(catch IOException ioe
nil)
(catch NumberFormatException nfe
nil)))

(defn configure-history
"Configure a MemoryHistory or FileHistory's max-size, if size
non-nil. Return nil."
[hist ^Integer size]
(when size
(.setMaxSize hist size)))

(defn setup-console-reader
[{:keys [prompt-string reader input-stream output-stream
history-file completer-factory blink-parens]
Expand All @@ -77,12 +97,16 @@
blink-parens true}
:as state}]
(let [reader (ConsoleReader. input-stream output-stream)
file-history (FileHistory. (make-history-file history-file))
hist-size (default-history-size nil) ;; currently null appName
file-history (doto (FileHistory. (make-history-file history-file) false)
(configure-history hist-size)
(.load))
history (try
(flush-history file-history)
file-history
(catch IOException e
(MemoryHistory.)))
(doto (MemoryHistory.)
(configure-history hist-size))))
completer (if completer-factory
(completer-factory reader)
nil)]
Expand Down