From e01b75330c562d945a765528a847584d96439431 Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Tue, 15 Oct 2024 13:51:02 -0700 Subject: [PATCH 01/20] Enable lexical binding --- js-comint.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js-comint.el b/js-comint.el index 6b3d845..1a56bed 100644 --- a/js-comint.el +++ b/js-comint.el @@ -1,4 +1,4 @@ -;;; js-comint.el --- JavaScript interpreter in window. +;;; js-comint.el --- JavaScript interpreter in window. -*- lexical-binding: t -*- ;;; Copyright (C) 2008 Paul Huff ;;; Copyright (C) 2015 Stefano Mazzucco From cc6a360415c85df02a49a1ef9058ba28e018ad6d Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Wed, 16 Oct 2024 20:23:10 -0700 Subject: [PATCH 02/20] Ignore string arg to js-comint-filter-output It just changes the output in the comint buffer. --- js-comint.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js-comint.el b/js-comint.el index 1a56bed..563d74b 100644 --- a/js-comint.el +++ b/js-comint.el @@ -273,8 +273,8 @@ before the process starts." (sit-for 1)) (js-comint-start-or-switch-to-repl)) -(defun js-comint-filter-output (string) - "Filter extra escape sequences from STRING." +(defun js-comint-filter-output (_string) + "Filter extra escape sequences from last output." (let ((beg (or comint-last-output-start (point-min-marker))) (end (process-mark (get-buffer-process (current-buffer))))) From ed7e77a3795fd1c5b5d45cbaef2a4a80a1570c34 Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Wed, 16 Oct 2024 20:54:12 -0700 Subject: [PATCH 03/20] Lint and simplify js-comint-repl require does not load if already present. Convert cond to equivalent if --- js-comint.el | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/js-comint.el b/js-comint.el index 563d74b..cec442d 100644 --- a/js-comint.el +++ b/js-comint.el @@ -319,7 +319,7 @@ before the process starts." (js-comint-setup-module-paths) (let* ((repl-mode (or (getenv "NODE_REPL_MODE") "magic")) (js-comint-code (format js-comint-code-format - (window-width) js-comint-prompt repl-mode))) + (window-width) js-comint-prompt repl-mode))) (pop-to-buffer (apply 'make-comint js-comint-buffer js-comint-program-command nil `(,@js-comint-program-arguments "-e" ,js-comint-code))) @@ -339,21 +339,20 @@ The environment variable \"NODE_PATH\" is setup by `js-comint-module-paths'." js-comint-program-command js-comint-program-arguments))) (when js-use-nvm - (unless (featurep 'nvm) (require 'nvm)) + (require 'nvm) (unless js-nvm-current-version (js-comint-select-node-version))) - (setq js-comint-program-arguments (split-string cmd)) + (setq js-comint-program-arguments (split-string cmd)) (setq js-comint-program-command (pop js-comint-program-arguments))))) - ;; set NOT_PATH automatically - (cond - ((and js-comint-set-env-when-startup - (file-exists-p (js-comint--suggest-module-path))) - (let* ((js-comint-module-paths (nconc (list (file-truename (js-comint--suggest-module-path))) - js-comint-module-paths))) - (js-comint-start-or-switch-to-repl))) - (t - (js-comint-start-or-switch-to-repl)))) + ;; set NODE_PATH automatically + (if (and js-comint-set-env-when-startup + (file-exists-p (js-comint--suggest-module-path))) + (let ((js-comint-module-paths (cons (file-truename (js-comint--suggest-module-path)) + js-comint-module-paths))) + (js-comint-start-or-switch-to-repl)) + ;; else + (js-comint-start-or-switch-to-repl))) (defalias 'run-js 'js-comint-repl) From d9088c32a4a06fbd8428d64b331488a01cddd230 Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Wed, 16 Oct 2024 14:09:48 -0700 Subject: [PATCH 04/20] Add lexical-binding and requires to test --- test-js-comint.el | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test-js-comint.el b/test-js-comint.el index e89adf9..b3e0236 100644 --- a/test-js-comint.el +++ b/test-js-comint.el @@ -1,3 +1,8 @@ +;; -*- lexical-binding: t -*- + +(require 'js-comint) +(require 'ert) + (defun js-comint-test-buffer-matches (regex) "Search the js-comint buffer for the given regular expression. Return 't if a match is found, nil otherwise." From ceeb9ebafb1dbe8eeb50fda92e2d0cf1a8a9d083 Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Thu, 17 Oct 2024 12:09:41 -0700 Subject: [PATCH 05/20] Bug: js-comint--suggest-module-path should be nil if no node_modules This means that default-directory is always added to NODE_PATH when calling js-comint-repl --- js-comint.el | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/js-comint.el b/js-comint.el index cec442d..7d4734f 100644 --- a/js-comint.el +++ b/js-comint.el @@ -188,12 +188,9 @@ Return a string representing the node version." (if (eq system-type 'windows-nt) ";" ":")) (defun js-comint--suggest-module-path () - "Find node_modules." - (let* ((dir (locate-dominating-file default-directory - "node_modules"))) - (if dir (concat (file-name-as-directory dir) - "node_modules") - default-directory))) + "Path to node_modules in parent dirs, or nil if none exists." + (when-let ((dir (locate-dominating-file default-directory "node_modules"))) + (concat (file-name-as-directory dir) "node_modules"))) (defun js-comint-get-process () "Get repl process." @@ -204,8 +201,9 @@ Return a string representing the node version." (defun js-comint-add-module-path () "Add a directory to `js-comint-module-paths'." (interactive) - (let* ((dir (read-directory-name "Module path:" - (js-comint--suggest-module-path)))) + (let ((dir (read-directory-name "Module path:" + (or (js-comint--suggest-module-path) + default-directory)))) (when dir (add-to-list 'js-comint-module-paths (file-truename dir)) (message "\"%s\" added to `js-comint-module-paths'" dir)))) @@ -347,7 +345,7 @@ The environment variable \"NODE_PATH\" is setup by `js-comint-module-paths'." ;; set NODE_PATH automatically (if (and js-comint-set-env-when-startup - (file-exists-p (js-comint--suggest-module-path))) + (js-comint--suggest-module-path)) (let ((js-comint-module-paths (cons (file-truename (js-comint--suggest-module-path)) js-comint-module-paths))) (js-comint-start-or-switch-to-repl)) From addff71a7ac958ea535a19f12dacb1442a15ba82 Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Thu, 17 Oct 2024 14:15:35 -0700 Subject: [PATCH 06/20] Use temporary local env vars when calling comint This avoids modifying NODE_PATH for later calls --- js-comint.el | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/js-comint.el b/js-comint.el index 7d4734f..d0ec399 100644 --- a/js-comint.el +++ b/js-comint.el @@ -310,17 +310,24 @@ before the process starts." ;;;###autoload -(defun js-comint-start-or-switch-to-repl () - "Start a new repl or switch to existing repl." +(defun js-comint-start-or-switch-to-repl (&optional node-modules-path) + "Start a new repl or switch to existing repl. +Optional NODE-MODULES-PATH is the path to a local node_modules to use." (interactive) - (setenv "NODE_NO_READLINE" "1") - (js-comint-setup-module-paths) - (let* ((repl-mode (or (getenv "NODE_REPL_MODE") "magic")) + (let* ((node-path (getenv "NODE_PATH")) + (all-paths-list (flatten-list (list node-path + node-modules-path + js-comint-module-paths))) + (local-node-path (string-join all-paths-list (js-comint--path-sep))) + (repl-mode (or (getenv "NODE_REPL_MODE") "magic")) (js-comint-code (format js-comint-code-format (window-width) js-comint-prompt repl-mode))) - (pop-to-buffer - (apply 'make-comint js-comint-buffer js-comint-program-command nil - `(,@js-comint-program-arguments "-e" ,js-comint-code))) + ;; TODO what happens if you switch into this from a different project? + (with-environment-variables (("NODE_NO_READLINE" "1") + ("NODE_PATH" local-node-path)) + (pop-to-buffer + (apply 'make-comint js-comint-buffer js-comint-program-command nil + `(,@js-comint-program-arguments "-e" ,js-comint-code)))) (js-comint-mode))) ;;;###autoload @@ -344,11 +351,9 @@ The environment variable \"NODE_PATH\" is setup by `js-comint-module-paths'." (setq js-comint-program-command (pop js-comint-program-arguments))))) ;; set NODE_PATH automatically - (if (and js-comint-set-env-when-startup - (js-comint--suggest-module-path)) - (let ((js-comint-module-paths (cons (file-truename (js-comint--suggest-module-path)) - js-comint-module-paths))) - (js-comint-start-or-switch-to-repl)) + (if-let ((module-path (and js-comint-set-env-when-startup + (js-comint--suggest-module-path)))) + (js-comint-start-or-switch-to-repl (file-truename module-path)) ;; else (js-comint-start-or-switch-to-repl))) From 3d879f595714b31fa47c5861a299a81a9d6ee51e Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Thu, 17 Oct 2024 14:18:11 -0700 Subject: [PATCH 07/20] Remove unused js-comint-setup-module-paths --- js-comint.el | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/js-comint.el b/js-comint.el index d0ec399..b93f20f 100644 --- a/js-comint.el +++ b/js-comint.el @@ -242,28 +242,10 @@ Return a string representing the node version." (t (message "Nothing to save. `js-comint-module-paths' is empty."))))) -(defun js-comint-setup-module-paths () - "Setup node_modules path." - (let* ((paths (mapconcat 'identity - js-comint-module-paths - (js-comint--path-sep))) - (node-path (getenv "NODE_PATH"))) - (cond - ((or (not node-path) - (string= "" node-path)) - ;; set - (setenv "NODE_PATH" paths)) - ((not (string= "" paths)) - ;; append - (setenv "NODE_PATH" (concat node-path (js-comint--path-sep) paths)) - (message "%s added into \$NODE_PATH" paths))))) - ;;;###autoload (defun js-comint-reset-repl () "Kill existing REPL process if possible. -Create a new Javascript REPL process. -The environment variable `NODE_PATH' is setup by `js-comint-module-paths' -before the process starts." +Create a new Javascript REPL process." (interactive) (when (js-comint-get-process) (process-send-string (js-comint-get-process) ".exit\n") @@ -332,8 +314,7 @@ Optional NODE-MODULES-PATH is the path to a local node_modules to use." ;;;###autoload (defun js-comint-repl (cmd) - "Start a Javascript process by running CMD. -The environment variable \"NODE_PATH\" is setup by `js-comint-module-paths'." + "Start a Javascript process by running CMD." (interactive (list ;; You can select node version here From 2eceaf37926919497941eb7a710894e272c44ecd Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Thu, 17 Oct 2024 14:21:26 -0700 Subject: [PATCH 08/20] Depend on emacs 28.1 to use with-environment-variables --- .github/workflows/actions.yml | 2 +- js-comint.el | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml index 28a94b3..306c453 100644 --- a/.github/workflows/actions.yml +++ b/.github/workflows/actions.yml @@ -4,7 +4,7 @@ jobs: test-emacs-versions: strategy: matrix: - emacs-version: [26, 27, 28, 29] + emacs-version: [28, 29] runs-on: ubuntu-latest container: silex/emacs:${{matrix.emacs-version}} steps: diff --git a/js-comint.el b/js-comint.el index b93f20f..610f980 100644 --- a/js-comint.el +++ b/js-comint.el @@ -9,7 +9,7 @@ ;;; Created: 15 Feb 2014 ;;; Version: 1.2.0 ;;; URL: https://github.com/redguardtoo/js-comint -;;; Package-Requires: ((emacs "24.3")) +;;; Package-Requires: ((emacs "28.1")) ;;; Keywords: javascript, node, inferior-mode, convenience ;; This file is NOT part of GNU Emacs. From 18ab6737f706db09199dd9d987d1a8e2509b28e1 Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Thu, 17 Oct 2024 14:57:50 -0700 Subject: [PATCH 09/20] Tidy argument handling for js-comint-repl Made cmd optional and documented behaviour. --- js-comint.el | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/js-comint.el b/js-comint.el index 610f980..57ef4d6 100644 --- a/js-comint.el +++ b/js-comint.el @@ -313,23 +313,30 @@ Optional NODE-MODULES-PATH is the path to a local node_modules to use." (js-comint-mode))) ;;;###autoload -(defun js-comint-repl (cmd) - "Start a Javascript process by running CMD." +(defun js-comint-repl (&optional cmd) + "Start a NodeJS REPL process. +Optional CMD will override `js-comint-program-command' and +`js-comint-program-arguments', as well as any nvm setting. + +When called interactively use a universal prefix to +set CMD." (interactive - (list - ;; You can select node version here - (when current-prefix-arg - (setq cmd - (read-string "Run js: " - (format "%s %s" - js-comint-program-command - js-comint-program-arguments))) - (when js-use-nvm - (require 'nvm) - (unless js-nvm-current-version (js-comint-select-node-version))) - - (setq js-comint-program-arguments (split-string cmd)) - (setq js-comint-program-command (pop js-comint-program-arguments))))) + (when current-prefix-arg + (list + (read-string "Run js: " + (string-join + (cons js-comint-program-command + js-comint-program-arguments) + " "))))) + + (when (and (not cmd) + js-use-nvm) + (require 'nvm) + (unless js-nvm-current-version (js-comint-select-node-version))) + + (when cmd + (setq js-comint-program-arguments (split-string cmd)) + (setq js-comint-program-command (pop js-comint-program-arguments))) ;; set NODE_PATH automatically (if-let ((module-path (and js-comint-set-env-when-startup @@ -398,7 +405,7 @@ If no region selected, you could manually input javascript expression." "Load FILE into the javascript interpreter." (interactive "f") (let ((file (expand-file-name file))) - (js-comint-repl js-comint-program-command) + (js-comint-repl) (comint-send-string (js-comint-get-process) (js-comint-guess-load-file-cmd file)))) ;;;###autoload From e83eb2703c9bbbfc32092b155151c71ebdb8ffda Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Thu, 17 Oct 2024 15:07:14 -0700 Subject: [PATCH 10/20] Add missing customization types to fix compiler warnings --- js-comint.el | 3 +++ 1 file changed, 3 insertions(+) diff --git a/js-comint.el b/js-comint.el index 57ef4d6..6384c63 100644 --- a/js-comint.el +++ b/js-comint.el @@ -92,10 +92,12 @@ (defcustom js-comint-program-command "node" "JavaScript interpreter." + :type 'string :group 'js-comint) (defcustom js-comint-set-env-when-startup t "Set environment variable NODE_PATH automatically during startup." + :type 'boolean :group 'js-comint) (defvar js-comint-module-paths '() @@ -107,6 +109,7 @@ (defcustom js-comint-program-arguments '() "List of command line arguments passed to the JavaScript interpreter." + :type '(list string) :group 'js-comint) (defcustom js-comint-prompt "> " From 2058be8736163c14abbc0edb23c9a341127a86eb Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Thu, 17 Oct 2024 15:23:02 -0700 Subject: [PATCH 11/20] Add unit tests for js-comint-start-or-switch-to-repl --- test-js-comint.el | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/test-js-comint.el b/test-js-comint.el index b3e0236..86a740e 100644 --- a/test-js-comint.el +++ b/test-js-comint.el @@ -47,3 +47,48 @@ DOS line separators." map((it) => it + 1). filter((it) => it > 0). reduce((prev, curr) => prev + curr, 0);" "^9$"))) + +(ert-deftest js-comint-start-or-switch-to-repl/test-no-modules () + "Should preserve node_path when nothing is set." + (let ((original js-comint-module-paths) + (original-env (getenv "NODE_PATH"))) + (unwind-protect + (progn + (setq js-comint-module-paths '()) + (setenv "NODE_PATH" "/foo/bar") + (js-comint-start-or-switch-to-repl) + (sit-for 1) + (js-comint-send-string "process.env['NODE_PATH'];") + (js-comint-test-buffer-matches "/foo/bar")) + (setq js-comint-module-paths original) + (setenv "NODE_PATH" original-env)))) + +(ert-deftest js-comint-start-or-switch-to-repl/test-global-set () + "Should include the value of `js-comint-node-modules' if set." + (let ((original js-comint-module-paths) + (original-env (getenv "NODE_PATH"))) + (unwind-protect + (progn + (setq js-comint-module-paths '("/baz/xyz")) + (setenv "NODE_PATH" "/foo/bar") + (js-comint-start-or-switch-to-repl) + (sit-for 1) + (js-comint-send-string "process.env['NODE_PATH'];") + (js-comint-test-buffer-matches (concat "/foo/bar" (js-comint--path-sep) "/baz/xyz"))) + (setq js-comint-module-paths original) + (setenv "NODE_PATH" original-env)))) + +(ert-deftest js-comint-start-or-switch-to-repl/test-local () + "Should include the optional node-modules-path." + (let ((original js-comint-module-paths) + (original-env (getenv "NODE_PATH"))) + (unwind-protect + (progn + (setq js-comint-module-paths '()) + (setenv "NODE_PATH" "/foo/bar") + (js-comint-start-or-switch-to-repl "/baz/xyz") + (sit-for 1) + (js-comint-send-string "process.env['NODE_PATH'];") + (js-comint-test-buffer-matches (concat "/foo/bar" (js-comint--path-sep) "/baz/xyz"))) + (setq js-comint-module-paths original) + (setenv "NODE_PATH" original-env)))) From dd6c11718310a84ec965c79b73f3f7e90013133d Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Thu, 17 Oct 2024 15:39:34 -0700 Subject: [PATCH 12/20] Simplify js-comint--suggest-module-path read-directory-name defaults to default-directory if nil and expand-file-name produces output equivalent to file-truename. --- js-comint.el | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/js-comint.el b/js-comint.el index 6384c63..d5a6708 100644 --- a/js-comint.el +++ b/js-comint.el @@ -193,7 +193,7 @@ Return a string representing the node version." (defun js-comint--suggest-module-path () "Path to node_modules in parent dirs, or nil if none exists." (when-let ((dir (locate-dominating-file default-directory "node_modules"))) - (concat (file-name-as-directory dir) "node_modules"))) + (expand-file-name "node_modules" dir))) (defun js-comint-get-process () "Get repl process." @@ -205,8 +205,7 @@ Return a string representing the node version." "Add a directory to `js-comint-module-paths'." (interactive) (let ((dir (read-directory-name "Module path:" - (or (js-comint--suggest-module-path) - default-directory)))) + (js-comint--suggest-module-path)))) (when dir (add-to-list 'js-comint-module-paths (file-truename dir)) (message "\"%s\" added to `js-comint-module-paths'" dir)))) @@ -341,12 +340,9 @@ set CMD." (setq js-comint-program-arguments (split-string cmd)) (setq js-comint-program-command (pop js-comint-program-arguments))) - ;; set NODE_PATH automatically - (if-let ((module-path (and js-comint-set-env-when-startup - (js-comint--suggest-module-path)))) - (js-comint-start-or-switch-to-repl (file-truename module-path)) - ;; else - (js-comint-start-or-switch-to-repl))) + ;; set NODE_PATH if js-comint-set-env-when-startup is true + (js-comint-start-or-switch-to-repl (and js-comint-set-env-when-startup + (js-comint--suggest-module-path)))) (defalias 'run-js 'js-comint-repl) From 951131ceb542f983f533b7c6086bb7e08f66ed0b Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Thu, 17 Oct 2024 16:01:07 -0700 Subject: [PATCH 13/20] Have js-comint-start-or-switch-to-repl check local node_modules This way js-rest-repl does not "forget" the local node_modules. --- js-comint.el | 12 ++++++------ test-js-comint.el | 46 +++++++++++++++++++++++++++++++++++----------- 2 files changed, 41 insertions(+), 17 deletions(-) diff --git a/js-comint.el b/js-comint.el index d5a6708..8d6128c 100644 --- a/js-comint.el +++ b/js-comint.el @@ -294,11 +294,13 @@ Create a new Javascript REPL process." ;;;###autoload -(defun js-comint-start-or-switch-to-repl (&optional node-modules-path) - "Start a new repl or switch to existing repl. -Optional NODE-MODULES-PATH is the path to a local node_modules to use." +(defun js-comint-start-or-switch-to-repl () + "Start a new repl or switch to existing repl." (interactive) (let* ((node-path (getenv "NODE_PATH")) + ;; The path to a local node_modules + (node-modules-path (and js-comint-set-env-when-startup + (js-comint--suggest-module-path))) (all-paths-list (flatten-list (list node-path node-modules-path js-comint-module-paths))) @@ -340,9 +342,7 @@ set CMD." (setq js-comint-program-arguments (split-string cmd)) (setq js-comint-program-command (pop js-comint-program-arguments))) - ;; set NODE_PATH if js-comint-set-env-when-startup is true - (js-comint-start-or-switch-to-repl (and js-comint-set-env-when-startup - (js-comint--suggest-module-path)))) + (js-comint-start-or-switch-to-repl)) (defalias 'run-js 'js-comint-repl) diff --git a/test-js-comint.el b/test-js-comint.el index 86a740e..d92739d 100644 --- a/test-js-comint.el +++ b/test-js-comint.el @@ -51,44 +51,68 @@ reduce((prev, curr) => prev + curr, 0);" "^9$"))) (ert-deftest js-comint-start-or-switch-to-repl/test-no-modules () "Should preserve node_path when nothing is set." (let ((original js-comint-module-paths) + (original-set-env js-comint-set-env-when-startup) (original-env (getenv "NODE_PATH"))) (unwind-protect (progn - (setq js-comint-module-paths '()) + (setq js-comint-module-paths nil + js-comint-set-env-when-startup nil) (setenv "NODE_PATH" "/foo/bar") + (when (js-comint-get-process) + (process-send-string (js-comint-get-process) ".exit\n") + (sit-for 1)) (js-comint-start-or-switch-to-repl) (sit-for 1) (js-comint-send-string "process.env['NODE_PATH'];") (js-comint-test-buffer-matches "/foo/bar")) - (setq js-comint-module-paths original) - (setenv "NODE_PATH" original-env)))) + (setq js-comint-module-paths original + js-comint-set-env-when-startup original-set-env) + (setenv "NODE_PATH" original-env) + (process-send-string (js-comint-get-process) ".exit\n")))) (ert-deftest js-comint-start-or-switch-to-repl/test-global-set () "Should include the value of `js-comint-node-modules' if set." (let ((original js-comint-module-paths) + (original-set-env js-comint-set-env-when-startup) (original-env (getenv "NODE_PATH"))) (unwind-protect (progn - (setq js-comint-module-paths '("/baz/xyz")) + (setq js-comint-module-paths '("/baz/xyz") + js-comint-set-env-when-startup nil) (setenv "NODE_PATH" "/foo/bar") + (when (js-comint-get-process) + (process-send-string (js-comint-get-process) ".exit\n") + (sit-for 1)) (js-comint-start-or-switch-to-repl) (sit-for 1) (js-comint-send-string "process.env['NODE_PATH'];") (js-comint-test-buffer-matches (concat "/foo/bar" (js-comint--path-sep) "/baz/xyz"))) - (setq js-comint-module-paths original) - (setenv "NODE_PATH" original-env)))) + (setq js-comint-module-paths original + js-comint-set-env-when-startup original-set-env) + (setenv "NODE_PATH" original-env) + (process-send-string (js-comint-get-process) ".exit\n")))) (ert-deftest js-comint-start-or-switch-to-repl/test-local () "Should include the optional node-modules-path." (let ((original js-comint-module-paths) - (original-env (getenv "NODE_PATH"))) + (original-set-env js-comint-set-env-when-startup) + (original-env (getenv "NODE_PATH")) + (original-suggest js-comint--suggest-module-path)) (unwind-protect (progn - (setq js-comint-module-paths '()) + (fset 'js-comint--suggest-module-path (lambda () "/baz/xyz")) + (setq js-comint-module-paths '() + js-comint-set-env-when-startup 't) (setenv "NODE_PATH" "/foo/bar") - (js-comint-start-or-switch-to-repl "/baz/xyz") + (when (js-comint-get-process) + (process-send-string (js-comint-get-process) ".exit\n") + (sit-for 1)) + (js-comint-start-or-switch-to-repl) (sit-for 1) (js-comint-send-string "process.env['NODE_PATH'];") (js-comint-test-buffer-matches (concat "/foo/bar" (js-comint--path-sep) "/baz/xyz"))) - (setq js-comint-module-paths original) - (setenv "NODE_PATH" original-env)))) + (setq js-comint-module-paths original + js-comint-set-env-when-startup original-set-env) + (setenv "NODE_PATH" original-env) + (fset 'js-comint--suggest-module-path original-suggest) + (process-send-string (js-comint-get-process) ".exit\n")))) From be24e8e381cdae9c9bb08a7440878f9ea02f0a4a Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Fri, 18 Oct 2024 08:53:22 -0700 Subject: [PATCH 14/20] Fix failing test --- test-js-comint.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-js-comint.el b/test-js-comint.el index d92739d..7ca7a87 100644 --- a/test-js-comint.el +++ b/test-js-comint.el @@ -97,7 +97,7 @@ reduce((prev, curr) => prev + curr, 0);" "^9$"))) (let ((original js-comint-module-paths) (original-set-env js-comint-set-env-when-startup) (original-env (getenv "NODE_PATH")) - (original-suggest js-comint--suggest-module-path)) + (original-suggest (symbol-function 'js-comint--suggest-module-path))) (unwind-protect (progn (fset 'js-comint--suggest-module-path (lambda () "/baz/xyz")) From 7ceec3d0c7b1f1e9a150bd40e7a828c231267060 Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Fri, 18 Oct 2024 08:56:30 -0700 Subject: [PATCH 15/20] Factor out process exit logic in tests --- test-js-comint.el | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test-js-comint.el b/test-js-comint.el index 7ca7a87..fff330c 100644 --- a/test-js-comint.el +++ b/test-js-comint.el @@ -25,6 +25,12 @@ Return 't if a match is found, nil otherwise." (js-comint-test-buffer-matches regex)) +(defun js-comint-test-exit-comint () + "Finish process." + (when (js-comint-get-process) + (process-send-string (js-comint-get-process) ".exit\n") + (sit-for 1))) + (ert-deftest js-comint-test-multiline-dotchain-line-start () "Test multiline statement with dots at beginning of lines." (should (js-comint-test-output-matches "[1, 2, 3] @@ -58,9 +64,7 @@ reduce((prev, curr) => prev + curr, 0);" "^9$"))) (setq js-comint-module-paths nil js-comint-set-env-when-startup nil) (setenv "NODE_PATH" "/foo/bar") - (when (js-comint-get-process) - (process-send-string (js-comint-get-process) ".exit\n") - (sit-for 1)) + (js-comint-test-exit-comint) (js-comint-start-or-switch-to-repl) (sit-for 1) (js-comint-send-string "process.env['NODE_PATH'];") @@ -68,7 +72,7 @@ reduce((prev, curr) => prev + curr, 0);" "^9$"))) (setq js-comint-module-paths original js-comint-set-env-when-startup original-set-env) (setenv "NODE_PATH" original-env) - (process-send-string (js-comint-get-process) ".exit\n")))) + (js-comint-test-exit-comint)))) (ert-deftest js-comint-start-or-switch-to-repl/test-global-set () "Should include the value of `js-comint-node-modules' if set." @@ -80,9 +84,7 @@ reduce((prev, curr) => prev + curr, 0);" "^9$"))) (setq js-comint-module-paths '("/baz/xyz") js-comint-set-env-when-startup nil) (setenv "NODE_PATH" "/foo/bar") - (when (js-comint-get-process) - (process-send-string (js-comint-get-process) ".exit\n") - (sit-for 1)) + (js-comint-test-exit-comint) (js-comint-start-or-switch-to-repl) (sit-for 1) (js-comint-send-string "process.env['NODE_PATH'];") @@ -90,7 +92,7 @@ reduce((prev, curr) => prev + curr, 0);" "^9$"))) (setq js-comint-module-paths original js-comint-set-env-when-startup original-set-env) (setenv "NODE_PATH" original-env) - (process-send-string (js-comint-get-process) ".exit\n")))) + (js-comint-test-exit-comint)))) (ert-deftest js-comint-start-or-switch-to-repl/test-local () "Should include the optional node-modules-path." @@ -104,9 +106,7 @@ reduce((prev, curr) => prev + curr, 0);" "^9$"))) (setq js-comint-module-paths '() js-comint-set-env-when-startup 't) (setenv "NODE_PATH" "/foo/bar") - (when (js-comint-get-process) - (process-send-string (js-comint-get-process) ".exit\n") - (sit-for 1)) + (js-comint-test-exit-comint) (js-comint-start-or-switch-to-repl) (sit-for 1) (js-comint-send-string "process.env['NODE_PATH'];") @@ -115,4 +115,4 @@ reduce((prev, curr) => prev + curr, 0);" "^9$"))) js-comint-set-env-when-startup original-set-env) (setenv "NODE_PATH" original-env) (fset 'js-comint--suggest-module-path original-suggest) - (process-send-string (js-comint-get-process) ".exit\n")))) + (js-comint-test-exit-comint)))) From 8f05c1b402efdca7b082cc20a515781ba42af7c2 Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Fri, 18 Oct 2024 11:23:26 -0700 Subject: [PATCH 16/20] Add declares for nvm functions. This fixes byte-compiler warnings, but keeps nvm optional --- js-comint.el | 3 +++ 1 file changed, 3 insertions(+) diff --git a/js-comint.el b/js-comint.el index 8d6128c..f872b5c 100644 --- a/js-comint.el +++ b/js-comint.el @@ -143,6 +143,9 @@ (defvar js-nvm-current-version nil "Current version of node.") +(declare-function nvm--installed-versions "nvm.el" ()) +(declare-function nvm--find-exact-version-for "nvm.el" (short)) + (defun js-comint-list-nvm-versions (prompt) "List all available node versions from nvm prompting the user with PROMPT. Return a string representing the node version." From 7fbcb4b18a95ed9becb9856117f00a1473e4aa48 Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Fri, 18 Oct 2024 11:32:32 -0700 Subject: [PATCH 17/20] Move (require 'nvm) into select-node-version, tidy logic js-comint-select-node-version is the common entry point when using nvm functions. --- js-comint.el | 45 ++++++++++++++++++++------------------------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/js-comint.el b/js-comint.el index f872b5c..40db58f 100644 --- a/js-comint.el +++ b/js-comint.el @@ -141,7 +141,8 @@ "require('repl')['REPL_MODE_' + '%s'.toUpperCase()])")) (defvar js-nvm-current-version nil - "Current version of node.") + "Current version of node for js-comint. +Either nil or a list (VERSION-STRING PATH).") (declare-function nvm--installed-versions "nvm.el" ()) (declare-function nvm--find-exact-version-for "nvm.el" (short)) @@ -164,21 +165,17 @@ Return a string representing the node version." (defun js-comint-select-node-version (&optional version) "Use a given VERSION of node from nvm." (interactive) - (if version - (setq js-nvm-current-version (nvm--find-exact-version-for version)) - (let ((old-js-nvm js-nvm-current-version)) - (setq js-nvm-current-version - (nvm--find-exact-version-for - (js-comint-list-nvm-versions - (if old-js-nvm - (format "Node version (current %s): " (car old-js-nvm)) - "Node version: ")))))) - (progn - (setq js-comint-program-command - (concat - (car (last js-nvm-current-version)) - "/bin" - "/node")))) + (require 'nvm) + (setq js-use-nvm t) ;; NOTE: js-use-nvm could probably be deprecated + (unless version + (let* ((old-js-nvm (car js-nvm-current-version)) + (prompt (if old-js-nvm + (format "Node version (current %s): " old-js-nvm) + "Node version: "))) + (setq version (js-comint-list-nvm-versions prompt)))) + + (setq js-nvm-current-version (nvm--find-exact-version-for version) + js-comint-program-command (format "%s/bin/node" (cadr js-nvm-current-version)))) (defun js-comint-guess-load-file-cmd (filename) "Create Node file loading command for FILENAME." @@ -335,15 +332,13 @@ set CMD." (cons js-comint-program-command js-comint-program-arguments) " "))))) - - (when (and (not cmd) - js-use-nvm) - (require 'nvm) - (unless js-nvm-current-version (js-comint-select-node-version))) - - (when cmd - (setq js-comint-program-arguments (split-string cmd)) - (setq js-comint-program-command (pop js-comint-program-arguments))) + (if cmd + (let ((cmd-parts (split-string cmd))) + (setq js-comint-program-arguments (cdr cmd-parts) + js-comint-program-command (car cmd-parts))) + (when (and js-use-nvm + (not js-nvm-current-version)) + (js-comint-select-node-version))) (js-comint-start-or-switch-to-repl)) From 1c3b1ab62227458b5245874bfe7e1b8b9ca9a7f7 Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Fri, 18 Oct 2024 11:43:22 -0700 Subject: [PATCH 18/20] Message when changing cmd while nvm is set --- js-comint.el | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/js-comint.el b/js-comint.el index 40db58f..ddc5a10 100644 --- a/js-comint.el +++ b/js-comint.el @@ -335,7 +335,10 @@ set CMD." (if cmd (let ((cmd-parts (split-string cmd))) (setq js-comint-program-arguments (cdr cmd-parts) - js-comint-program-command (car cmd-parts))) + js-comint-program-command (car cmd-parts)) + (when js-nvm-current-version + (message "nvm node version overridden, reset with M-x js-comint-select-node-version") + (setq js-use-nvm nil))) (when (and js-use-nvm (not js-nvm-current-version)) (js-comint-select-node-version))) From 6eaa359a8761266d17ae46df1f5e6e65d7f29df7 Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Fri, 18 Oct 2024 12:22:02 -0700 Subject: [PATCH 19/20] Do not include empty strings in path --- js-comint.el | 1 + 1 file changed, 1 insertion(+) diff --git a/js-comint.el b/js-comint.el index ddc5a10..25a8dfa 100644 --- a/js-comint.el +++ b/js-comint.el @@ -304,6 +304,7 @@ Create a new Javascript REPL process." (all-paths-list (flatten-list (list node-path node-modules-path js-comint-module-paths))) + (all-paths-list (seq-remove 'string-empty-p all-paths-list)) (local-node-path (string-join all-paths-list (js-comint--path-sep))) (repl-mode (or (getenv "NODE_REPL_MODE") "magic")) (js-comint-code (format js-comint-code-format From 4353fe2fa6f0bea8c005ae2ee9d5c1e6597da81d Mon Sep 17 00:00:00 2001 From: Josh Bax Date: Fri, 18 Oct 2024 12:24:53 -0700 Subject: [PATCH 20/20] Switch buffers in start-or-switch-to-buffer --- js-comint.el | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/js-comint.el b/js-comint.el index 25a8dfa..22bf916 100644 --- a/js-comint.el +++ b/js-comint.el @@ -297,25 +297,26 @@ Create a new Javascript REPL process." (defun js-comint-start-or-switch-to-repl () "Start a new repl or switch to existing repl." (interactive) - (let* ((node-path (getenv "NODE_PATH")) - ;; The path to a local node_modules - (node-modules-path (and js-comint-set-env-when-startup - (js-comint--suggest-module-path))) - (all-paths-list (flatten-list (list node-path - node-modules-path - js-comint-module-paths))) - (all-paths-list (seq-remove 'string-empty-p all-paths-list)) - (local-node-path (string-join all-paths-list (js-comint--path-sep))) - (repl-mode (or (getenv "NODE_REPL_MODE") "magic")) - (js-comint-code (format js-comint-code-format - (window-width) js-comint-prompt repl-mode))) - ;; TODO what happens if you switch into this from a different project? - (with-environment-variables (("NODE_NO_READLINE" "1") - ("NODE_PATH" local-node-path)) - (pop-to-buffer - (apply 'make-comint js-comint-buffer js-comint-program-command nil - `(,@js-comint-program-arguments "-e" ,js-comint-code)))) - (js-comint-mode))) + (if (js-comint-get-process) + (pop-to-buffer (js-comint-get-buffer)) + (let* ((node-path (getenv "NODE_PATH")) + ;; The path to a local node_modules + (node-modules-path (and js-comint-set-env-when-startup + (js-comint--suggest-module-path))) + (all-paths-list (flatten-list (list node-path + node-modules-path + js-comint-module-paths))) + (all-paths-list (seq-remove 'string-empty-p all-paths-list)) + (local-node-path (string-join all-paths-list (js-comint--path-sep))) + (repl-mode (or (getenv "NODE_REPL_MODE") "magic")) + (js-comint-code (format js-comint-code-format + (window-width) js-comint-prompt repl-mode))) + (with-environment-variables (("NODE_NO_READLINE" "1") + ("NODE_PATH" local-node-path)) + (pop-to-buffer + (apply 'make-comint js-comint-buffer js-comint-program-command nil + `(,@js-comint-program-arguments "-e" ,js-comint-code)))) + (js-comint-mode)))) ;;;###autoload (defun js-comint-repl (&optional cmd)