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

Replace rlwrap with rlfe #230

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alexander-yakushev
Copy link
Contributor

@alexander-yakushev alexander-yakushev commented Apr 24, 2024

While working on #229, I discovered the following: the installation of rlwrap on Debian and Ubuntu pulls ~80 MB of additional packages (python3 and other stuff). That sounds excessive for a mere readline wrapper. Given that, and the fact that rlwrap is bugged in Docker and requires hacks to operate, this is a proposal to replace it with another readline wrapper, rlfe. It has a much slimmer dependency tree and looks simpler. I've verified that it works, supports GNU readline's navigation commands (^A, ^E, etc.), handles ^C dorrectly.

Main points:

  1. Interactive REPL is unlikely to be the "production" usecase for a Docker image. So, as long as we continue to provide the user-facing functionality in a similar way (GNU readine is still used), small differences shouldn't be a dealbreaker.
  2. The primary way of non-interactive invoking Clojure scripts, clojure, is not affected in any way.
  3. I also discovered that the entrypoint in current Alpine images seems to be broken because we didn't install rlwrap there before. Also, rlfe is not available on Alpine and will not be installed with these changes. However, the new wrapper script invokes clojuredirectly if rlfe is not found – a better behavior than what we have now, where calling clj on Alpine fails completely.

@alexander-yakushev
Copy link
Contributor Author

Drafting this for now. Entrypoint business is a mess 😞.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant