Skip to content

[Fix #116] Set inf-clojure-buffer REPL type on detect #119

New issue

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

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

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jan 1, 2018

Conversation

arichiardi
Copy link
Contributor

@arichiardi arichiardi commented Dec 30, 2017

Now the code sets the inf-clojure-repl-type buffer local var in the inf-clojure-buffer when a REPL type is detected. This solves the weird errors that were happening when working within the REPL buffer because inf-clojure-repl-type is nil.


Before submitting the PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):

  • The commits are consistent with our [contribution guidelines][1]
  • The new code is not generating bytecode or M-x checkdoc warnings
  • You've updated the changelog (if adding/changing user-visible functionality)

@arichiardi arichiardi force-pushed the set-repl-buffer-repl-type branch from f01e1c5 to d18dd8d Compare December 30, 2017 03:28
@arichiardi
Copy link
Contributor Author

This was indeed easy, style can be improved because setq returns the value so everything can be threaded but it looks okay this way too.

inf-clojure.el Outdated
(let ((detected-type (inf-clojure--detect-repl-type proc)))
(with-current-buffer inf-clojure-buffer
(setq inf-clojure-repl-type detected-type))
(setq inf-clojure-repl-type detected-type))
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to set the REPL type in the source buffer? We can always infer this from the REPL buffer for the project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uhm are there any implications in terms of performance in getting the buffer and from it the value for every operation? Remember this is done on each key press (for completion). If not I can change things, but this patch was definitely easier.

I think also that removing the buffer local would work but I was thinking that at some point we should have more than one repl (per project) so I don't know how it work then.

@arichiardi arichiardi force-pushed the set-repl-buffer-repl-type branch from d18dd8d to b401c82 Compare December 31, 2017 04:01
@arichiardi
Copy link
Contributor Author

I updated the code and also notice that we have a defvar-local that can be manually set. As long as it is not nil detection will be skipped.

@arichiardi arichiardi force-pushed the set-repl-buffer-repl-type branch from b401c82 to 4b5d5df Compare December 31, 2017 04:35
@arichiardi arichiardi changed the title [Fix #116] Set *inf-clojure* REPL type on detect [Fix #116] Set inf-clojure-buffer REPL type on detect Dec 31, 2017
@arichiardi arichiardi force-pushed the set-repl-buffer-repl-type branch from 4b5d5df to 22f80c4 Compare December 31, 2017 05:34
CHANGELOG.md Outdated
@@ -8,6 +8,7 @@
* [#83](https://github.com/clojure-emacs/inf-clojure/pull/85): No such namespace: complete.core in lumo REPL.
* [#93](https://github.com/clojure-emacs/inf-clojure/pull/93): Slow response from inf-clojure (completions, arglists, ...).
* [#101](https://github.com/clojure-emacs/inf-clojure/pull/101): `inf-clojure-set-ns` hangs Emacs.
* [#119](https://github.com/clojure-emacs/inf-clojure/pull/119): Send REPL string always, even if empty.
Copy link
Member

Choose a reason for hiding this comment

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

I think you put to wrong changelog entry here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep I renamed things and forgot it, will do

@arichiardi arichiardi force-pushed the set-repl-buffer-repl-type branch from 22f80c4 to d6daf9d Compare December 31, 2017 19:26
Now the code sets the inf-clojure-repl-type buffer local var in the
inf-clojure-buffer when a REPL type is detected. This solves the weird errors
that were happening when working within the REPL buffer because
inf-clojure-repl-type is nil.
@arichiardi arichiardi force-pushed the set-repl-buffer-repl-type branch from d6daf9d to 829fbe6 Compare December 31, 2017 20:41
@bbatsov bbatsov merged commit 5c8f590 into clojure-emacs:master Jan 1, 2018
@arichiardi arichiardi deleted the set-repl-buffer-repl-type branch January 2, 2018 02:53
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants