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

Cleanup socket repl startup. #212

Merged
merged 5 commits into from Apr 8, 2023

Conversation

dpsutton
Copy link
Contributor

@dpsutton dpsutton commented Apr 7, 2023

Fixes: #211

How to use:

To use one of the defaults defined in
inf-clojure-socket-repl-startup-forms, just m-x inf-clojure-socket-repl and choose your startup.

To add your own custom startup

(setq inf-clojure-custom-startup
      "clojure -J-Dclojure.server.repl=\"{:port %d :accept clojure.core.server/repl}\" -A:grepl")

This will prompt you for a repl type so it's useful to also include

(setq inf-clojure-custom-repl-type 'clojure)

Or set these in a .dir-locals.el file for each project with

;;; Directory Local Variables
;;; For more information see (info "(emacs) Directory Variables")

((nil . ((inf-clojure-custom-startup . "clojure -J-Dclojure.server.repl=\"{:port %d :accept clojure.core.server/repl}\" -A:grepl")
         (inf-clojure-custom-repl-type . clojure))))

things i changed

  • added (defvar inf-clojure-custom-repl-name nil). This was originally used but never defined so socket startup and inf-connect would fail. This also was not honored and now controls the repl buffer name.
  • removed inf-clojure-socket-repl-type. This was also used and never defined so it also caused startup to fail. But we already have a notion of inf-clojure-custom-repl-type so we can just reuse that. There's nothing special about it being the socket repl type for connect.
  • simplified the buffer startup mechanism. Lots of let bindings
  • added inf-clojure--suppress-connect-message-p. Ostensibly private, others can use it. But two things are sending messages: how we start the clojure process, and that we connected to the clojure process. I think how we start the clojure process is the far more important one as it includes port information, aliases, etc.

cc: @jakub-stastny @mikepjb

** How to use: **

To use one of the defaults defined in
`inf-clojure-socket-repl-startup-forms`, just `m-x
inf-clojure-socket-repl` and choose your startup.

To add your own custom startup

```emacs-lisp
(setq inf-clojure-custom-startup
      "clojure -J-Dclojure.server.repl=\"{:port %d :accept clojure.core.server/repl}\" -A:grepl")
```

This will prompt you for a repl type so it's useful to also include

```emacs-lisp
(setq inf-clojure-custom-repl-type 'clojure)
```

Or set these in a .dir-locals.el file for each project with

```emacs-lisp
;;; Directory Local Variables
;;; For more information see (info "(emacs) Directory Variables")

((nil . ((inf-clojure-custom-startup . "clojure -J-Dclojure.server.repl=\"{:port %d :accept clojure.core.server/repl}\" -A:grepl")
         (inf-clojure-custom-repl-type . clojure))))
```

** things i changed **
- added `(defvar inf-clojure-custom-repl-name nil)`. This was originally
used but never defined so socket startup and inf-connect would
fail. This also was not honored and now controls the repl buffer name.
- removed `inf-clojure-socket-repl-type`. This was also used and never
defined so it also caused startup to fail. But we already have a notion
of `inf-clojure-custom-repl-type` so we can just reuse that. There's
nothing special about it being the socket repl type for connect.
- simplified the buffer startup mechanism. Lots of `let` bindings
- added `inf-clojure--suppress-connect-message-p`. Ostensibly private,
others can use it. But two things are sending messages: how we start the
clojure process, and that we connected to the clojure process. I think
how we start the clojure process is the far more important one as it
includes port information, aliases, etc.
inf-clojure.el Outdated Show resolved Hide resolved
inf-clojure.el Outdated Show resolved Hide resolved
@bbatsov
Copy link
Member

bbatsov commented Apr 7, 2023

Seems I didn't pay close enough attention the PR that introduced those changes. My bad! Your PR looks good to me overall, apart from my small comments. Also I've noticed the following in the CI log:

[00:04.474]  
             In toplevel form:
             inf-clojure.el:614:1: Warning: custom-declare-variable `inf-clojure-auto-mode'
                 docstring wider than 80 characters
[00:04.501]  
             In inf-clojure-socket-repl-sentinel:
             inf-clojure.el:896:50: Warning: Unused lexical argument `event'

Seems those warnings got obscured by the test that has been failing for a while. Probably we should comment it out at this point.

@dpsutton
Copy link
Contributor Author

dpsutton commented Apr 7, 2023

I want to fix up all of the lint errors but I really don't understand the one that is complaining about the regex "^\\( *#_\\|[^=> \n]+\\)=> *". Also one complains that we have to depend on emacs 28.3 if we use eldoc which seems obviously wrong.

inf-clojure.el Outdated Show resolved Hide resolved
@dpsutton
Copy link
Contributor Author

dpsutton commented Apr 7, 2023

full lint run:

inf-clojure on  master [$?] on ☁️  metabase-query
❯ eldev lint
Bootstrapping Eldev for Emacs 28.2 from MELPA Stable...

inf-clojure.el:1142: Keycode C-u embedded in doc string.  Use \\<mapvar> & \\[command] instead
Found 1 warning in file ‘inf-clojure.el’
[1/1] Installing package ‘package-lint’ (0.17) from ‘melpa-stable’...
inf-clojure.el:1:0: warning: Including "Emacs" in the package summary is usually redundant.
inf-clojure.el:1103:0: error: `inf-clojure-prev-l/c-dir/file' contains a non-standard separator `/', use hyphens instead (see Elisp Coding Conventions).
inf-clojure.el:1374:10: error: You should depend on (emacs "28.1") if you need `eldoc'.
Found 3 warnings in file ‘inf-clojure.el’
[1/2] Installing package ‘xr’ (1.23) from ‘gnu’...
[2/2] Installing package ‘relint’ (1.22) from ‘gnu’...
inf-clojure.el:1505:49: In call to skip-chars-backward: Duplicated character ‘[’ (pos 16)
  "^[] \"'`><,;|&{()[@\\^]"
   .................^
inf-clojure.el:1505:49: In call to skip-chars-backward: Duplicated character ‘]’ (pos 20)
  "^[] \"'`><,;|&{()[@\\^]"
   ......................^
Found 2 warnings in file ‘inf-clojure.el’
Linting produced warnings

inf-clojure.el Outdated Show resolved Hide resolved
@bbatsov bbatsov merged commit b153e51 into clojure-emacs:master Apr 8, 2023
2 of 7 checks passed
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.

Symbol's value as variable is void: inf-clojure-custom-repl-name
2 participants