Skip to content

Conversation

@boehm-s
Copy link
Contributor

@boehm-s boehm-s commented Oct 4, 2018

A while ago, I answered this question, and I think the command could be useful to other users.

I'm not experimented at all in elisp, so I'm open to all kind of suggestions,

Cheers

@abicky
Copy link
Owner

abicky commented Oct 14, 2018

Thank you for your PR!
I think it is better to introduce nodejs-repl-use-global custom variable so that .clear command clears the current context. The current implementation sets useGlobal to true as node CLI REPL does.
cf. https://nodejs.org/api/repl.html#repl_repl_start_options

What do you think?

% node -e "require('repl').start({ replMode: 'magic', useGlobal: true })"
> let foo = 1;
undefined
> .clear
> let foo = 2;
TypeError: Identifier 'foo' has already been declared
> 
% node -e "require('repl').start({ replMode: 'magic', useGlobal: false })"
> let foo = 1;
undefined
> .clear
Clearing context...
> let foo = 2;
undefined

@boehm-s
Copy link
Contributor Author

boehm-s commented Oct 15, 2018

I haven't tought about that, it's way better, and cleaner for sure !

@boehm-s
Copy link
Contributor Author

boehm-s commented Oct 16, 2018

Do you think it's OK like that ?

nodejs-repl.el Outdated
(defvar nodejs-repl-code-format
(concat
"require('repl').start('%s', null, null, true, false, "
"require('repl').start({prompt: '%s', useGlobal: %s}, null, null, true, false, "
Copy link
Owner

Choose a reason for hiding this comment

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

repl.start has two signatures, one is repl.start(options) and another is repl.start(prompt, stream, eval, useGlobal, ignoreUndefined, replMode). If you specify a object as the first argument, other arguments are ignored, so please change the code like below:

diff --git a/nodejs-repl.el b/nodejs-repl.el
index 58834bb..32f758c 100644
--- a/nodejs-repl.el
+++ b/nodejs-repl.el
@@ -139,8 +139,8 @@ See also `comint-process-echoes'"

 (defvar nodejs-repl-code-format
   (concat
-   "require('repl').start({prompt: '%s', useGlobal: %s}, null, null, true, false, "
-   "require('repl')['REPL_MODE_' + '%s'.toUpperCase()])"))
+   "require('repl').start({ prompt: '%s', useGlobal: %s, replMode: "
+   "require('repl')['REPL_MODE_' + '%s'.toUpperCase()] })"))

 (defvar nodejs-repl-extra-espace-sequence-re "\\(\x1b\\[[0-9]+[GJK]\\)")

Copy link
Owner

@abicky abicky left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution :)

@abicky abicky merged commit 285228a into abicky:develop Oct 24, 2018
@abicky
Copy link
Owner

abicky commented Oct 24, 2018

I've published 0.2.1.

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.

2 participants