* bug#65162: 30.0.50; [PATCH] Simplify some internal Eshell command parsing functions
@ 2023-08-09 0:16 Jim Porter
2023-08-10 17:57 ` Jim Porter
0 siblings, 1 reply; 2+ messages in thread
From: Jim Porter @ 2023-08-09 0:16 UTC (permalink / raw)
To: 65162
[-- Attachment #1: Type: text/plain, Size: 444 bytes --]
These changes are just there for cleanup, and to make it easier to work
with this code going forward. Since Eshell command parsing now has
fairly-thorough regression tests, I think these changes should be pretty
safe.
In the second patch, I removed the need to use a dynamically-bound
variable, but kept the old (now obsolete) function around for
compatibility. I doubt any external packages are using this, but better
safe than sorry...
[-- Attachment #2: 0001-Simplify-command-parsing-in-Eshell.patch --]
[-- Type: text/plain, Size: 5332 bytes --]
From 318d09fb6ba7090c35147741e046924f61983342 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Mon, 7 Aug 2023 22:15:18 -0700
Subject: [PATCH 1/2] Simplify command parsing in Eshell
* lisp/eshell/esh-cmd.el (eshell-parse-command): Do all modifications
to each command in a single pass.
(eshell-parse-pipeline): Remove unncessary reversing of parsed
results.
---
lisp/eshell/esh-cmd.el | 92 +++++++++++++++++-------------------------
1 file changed, 38 insertions(+), 54 deletions(-)
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 94aa2ed8906..bd00e9f5406 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -416,26 +416,19 @@ eshell-parse-command
(commands
(mapcar
(lambda (cmd)
- (setq cmd
- (if (or (not (car eshell--sep-terms))
- (string= (car eshell--sep-terms) ";"))
- (eshell-parse-pipeline cmd)
- `(eshell-do-subjob
- (cons :eshell-background
- ,(eshell-parse-pipeline cmd)))))
- (setq eshell--sep-terms (cdr eshell--sep-terms))
- (if eshell-in-pipeline-p
- cmd
- `(eshell-trap-errors ,cmd)))
+ (let ((sep (pop eshell--sep-terms)))
+ (setq cmd (eshell-parse-pipeline cmd))
+ (when (equal sep "&")
+ (setq cmd `(eshell-do-subjob (cons :eshell-background ,cmd))))
+ (unless eshell-in-pipeline-p
+ (setq cmd `(eshell-trap-errors ,cmd)))
+ ;; Copy I/O handles so each full statement can manipulate
+ ;; them if they like. Steal the handles for the last
+ ;; command in the list; we won't use the originals again
+ ;; anyway.
+ (setq cmd `(eshell-with-copied-handles ,cmd ,(not sep)))
+ cmd))
(eshell-separate-commands terms "[&;]" nil 'eshell--sep-terms))))
- (let ((cmd commands))
- (while cmd
- ;; Copy I/O handles so each full statement can manipulate them
- ;; if they like. Steal the handles for the last command in
- ;; the list; we won't use the originals again anyway.
- (setcar cmd `(eshell-with-copied-handles
- ,(car cmd) ,(not (cdr cmd))))
- (setq cmd (cdr cmd))))
(if toplevel
`(eshell-commands (progn
(run-hooks 'eshell-pre-command-hook)
@@ -638,46 +631,37 @@ eshell-parse-pipeline
(let* (eshell--sep-terms
(bigpieces (eshell-separate-commands terms "\\(&&\\|||\\)"
nil 'eshell--sep-terms))
- (bp bigpieces)
- (results (list t))
- final)
- (while bp
- (let ((subterms (car bp)))
- (let* ((pieces (eshell-separate-commands subterms "|"))
- (p pieces))
- (while p
- (let ((cmd (car p)))
- (run-hook-with-args 'eshell-pre-rewrite-command-hook cmd)
- (setq cmd (run-hook-with-args-until-success
- 'eshell-rewrite-command-hook cmd))
- (let ((eshell--cmd cmd))
- (run-hook-with-args 'eshell-post-rewrite-command-hook
- 'eshell--cmd)
- (setq cmd eshell--cmd))
- (setcar p (funcall eshell-post-rewrite-command-function cmd)))
- (setq p (cdr p)))
- (nconc results
- (list
- (if (<= (length pieces) 1)
- (car pieces)
- (cl-assert (not eshell-in-pipeline-p))
- `(eshell-execute-pipeline (quote ,pieces))))))
- (setq bp (cdr bp))))
+ results final)
+ (dolist (subterms bigpieces)
+ (let* ((pieces (eshell-separate-commands subterms "|"))
+ (p pieces))
+ (while p
+ (let ((cmd (car p)))
+ (run-hook-with-args 'eshell-pre-rewrite-command-hook cmd)
+ (setq cmd (run-hook-with-args-until-success
+ 'eshell-rewrite-command-hook cmd))
+ (let ((eshell--cmd cmd))
+ (run-hook-with-args 'eshell-post-rewrite-command-hook
+ 'eshell--cmd)
+ (setq cmd eshell--cmd))
+ (setcar p (funcall eshell-post-rewrite-command-function cmd)))
+ (setq p (cdr p)))
+ (push (if (<= (length pieces) 1)
+ (car pieces)
+ (cl-assert (not eshell-in-pipeline-p))
+ `(eshell-execute-pipeline (quote ,pieces)))
+ results)))
;; `results' might be empty; this happens in the case of
;; multi-line input
- (setq results (cdr results)
- results (nreverse results)
- final (car results)
- results (cdr results)
- eshell--sep-terms (nreverse eshell--sep-terms))
+ (setq final (car results)
+ results (cdr results)
+ eshell--sep-terms (nreverse eshell--sep-terms))
(while results
(cl-assert (car eshell--sep-terms))
(setq final (eshell-structure-basic-command
- 'if (string= (car eshell--sep-terms) "&&") "if"
- `(eshell-protect ,(car results))
- `(eshell-protect ,final))
- results (cdr results)
- eshell--sep-terms (cdr eshell--sep-terms)))
+ 'if (string= (pop eshell--sep-terms) "&&") "if"
+ `(eshell-protect ,(pop results))
+ `(eshell-protect ,final))))
final))
(defun eshell-parse-subcommand-argument ()
--
2.25.1
[-- Attachment #3: 0002-Return-separators-from-eshell-split-commands-directl.patch --]
[-- Type: text/plain, Size: 8418 bytes --]
From 99fac65da8e0e96fca8290e9e481a7a97a70cbf4 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Mon, 7 Aug 2023 22:28:24 -0700
Subject: [PATCH 2/2] Return separators from 'eshell-split-commands' directly
when requested
This eliminates the need for using a dynamically-bound variable to
hold the list of separators.
* lisp/eshell/esh-cmd.el (eshell--sep-terms): Remove.
(eshell-split-commands): New function, adapted from
'eshell-separate-commands'.
(eshell-separate-commands): Make obsolete, and call
'eshell-split-commands'.
(eshell-parse-command, eshell-parse-pipeline): Use
'eshell-split-commands'.
* lisp/eshell/esh-arg.el (eshell-parse-delimiter): Update comment.
---
lisp/eshell/esh-arg.el | 2 +-
lisp/eshell/esh-cmd.el | 130 ++++++++++++++++++++++-------------------
2 files changed, 72 insertions(+), 60 deletions(-)
diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index aa1e8f77ea5..26be1127880 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -541,7 +541,7 @@ eshell-parse-special-reference
(defun eshell-parse-delimiter ()
"Parse an argument delimiter, which is essentially a command operator."
;; this `eshell-operator' keyword gets parsed out by
- ;; `eshell-separate-commands'. Right now the only possibility for
+ ;; `eshell-split-commands'. Right now the only possibility for
;; error is an incorrect output redirection specifier.
(when (looking-at "[&|;\n]\\s-*")
(let ((end (match-end 0)))
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index bd00e9f5406..80066263396 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -364,8 +364,6 @@ eshell-complete-lisp-symbols
;; Command parsing
-(defvar eshell--sep-terms)
-
(defmacro eshell-with-temp-command (region &rest body)
"Narrow the buffer to REGION and execute the forms in BODY.
@@ -404,31 +402,33 @@ eshell-parse-command
region. TOPLEVEL, if non-nil, means that the outermost command (the
user's input command) is being parsed, and that pre and post command
hooks should be run before and after the command."
- (let* (eshell--sep-terms
- (terms
- (append
- (if (consp command)
- (eshell-parse-arguments (car command) (cdr command))
- (eshell-with-temp-command command
- (goto-char (point-max))
- (eshell-parse-arguments (point-min) (point-max))))
- args))
- (commands
- (mapcar
- (lambda (cmd)
- (let ((sep (pop eshell--sep-terms)))
- (setq cmd (eshell-parse-pipeline cmd))
- (when (equal sep "&")
- (setq cmd `(eshell-do-subjob (cons :eshell-background ,cmd))))
- (unless eshell-in-pipeline-p
- (setq cmd `(eshell-trap-errors ,cmd)))
- ;; Copy I/O handles so each full statement can manipulate
- ;; them if they like. Steal the handles for the last
- ;; command in the list; we won't use the originals again
- ;; anyway.
- (setq cmd `(eshell-with-copied-handles ,cmd ,(not sep)))
- cmd))
- (eshell-separate-commands terms "[&;]" nil 'eshell--sep-terms))))
+ (pcase-let*
+ ((terms
+ (append
+ (if (consp command)
+ (eshell-parse-arguments (car command) (cdr command))
+ (eshell-with-temp-command command
+ (goto-char (point-max))
+ (eshell-parse-arguments (point-min) (point-max))))
+ args))
+ (`(,sub-chains . ,sep-terms)
+ (eshell-split-commands terms "[&;]" nil t))
+ (commands
+ (mapcar
+ (lambda (cmd)
+ (let ((sep (pop sep-terms)))
+ (setq cmd (eshell-parse-pipeline cmd))
+ (when (equal sep "&")
+ (setq cmd `(eshell-do-subjob (cons :eshell-background ,cmd))))
+ (unless eshell-in-pipeline-p
+ (setq cmd `(eshell-trap-errors ,cmd)))
+ ;; Copy I/O handles so each full statement can manipulate
+ ;; them if they like. Steal the handles for the last
+ ;; command in the list; we won't use the originals again
+ ;; anyway.
+ (setq cmd `(eshell-with-copied-handles ,cmd ,(not sep)))
+ cmd))
+ sub-chains)))
(if toplevel
`(eshell-commands (progn
(run-hooks 'eshell-pre-command-hook)
@@ -628,12 +628,12 @@ eshell--cmd
(defun eshell-parse-pipeline (terms)
"Parse a pipeline from TERMS, return the appropriate Lisp forms."
- (let* (eshell--sep-terms
- (bigpieces (eshell-separate-commands terms "\\(&&\\|||\\)"
- nil 'eshell--sep-terms))
- results final)
+ (pcase-let*
+ ((`(,bigpieces . ,sep-terms)
+ (eshell-split-commands terms "\\(&&\\|||\\)" nil t))
+ (results) (final))
(dolist (subterms bigpieces)
- (let* ((pieces (eshell-separate-commands subterms "|"))
+ (let* ((pieces (eshell-split-commands subterms "|"))
(p pieces))
(while p
(let ((cmd (car p)))
@@ -655,11 +655,11 @@ eshell-parse-pipeline
;; multi-line input
(setq final (car results)
results (cdr results)
- eshell--sep-terms (nreverse eshell--sep-terms))
+ sep-terms (nreverse sep-terms))
(while results
- (cl-assert (car eshell--sep-terms))
+ (cl-assert (car sep-terms))
(setq final (eshell-structure-basic-command
- 'if (string= (pop eshell--sep-terms) "&&") "if"
+ 'if (string= (pop sep-terms) "&&") "if"
`(eshell-protect ,(pop results))
`(eshell-protect ,final))))
final))
@@ -696,6 +696,34 @@ eshell-parse-lisp-argument
(eshell-lisp-command (quote ,obj)))
(ignore (goto-char here))))))
+(defun eshell-split-commands (terms separator &optional
+ reversed return-seps)
+ "Split TERMS using SEPARATOR.
+If REVERSED is non-nil, the list of separated term groups will be
+returned in reverse order.
+
+If RETURN-SEPS is nil, return just the separated terms as a list;
+otherwise, return both the separated terms and their separators
+as a pair of lists."
+ (let (sub-chains sub-terms sep-terms)
+ (dolist (term terms)
+ (if (and (eq (car-safe term) 'eshell-operator)
+ (string-match (concat "^" separator "$")
+ (nth 1 term)))
+ (progn
+ (push (nth 1 term) sep-terms)
+ (push (nreverse sub-terms) sub-chains)
+ (setq sub-terms nil))
+ (push term sub-terms)))
+ (when sub-terms
+ (push (nreverse sub-terms) sub-chains))
+ (unless reversed
+ (setq sub-chains (nreverse sub-chains)
+ sep-terms (nreverse sep-terms)))
+ (if return-seps
+ (cons sub-chains sep-terms)
+ sub-chains)))
+
(defun eshell-separate-commands (terms separator &optional
reversed last-terms-sym)
"Separate TERMS using SEPARATOR.
@@ -703,30 +731,14 @@ eshell-separate-commands
returned in reverse order. If LAST-TERMS-SYM is a symbol, its value
will be set to a list of all the separator operators found (or (nil)
if none)."
- (let ((sub-terms (list t))
- (eshell-sep-terms (list t))
- subchains)
- (while terms
- (if (and (consp (car terms))
- (eq (caar terms) 'eshell-operator)
- (string-match (concat "^" separator "$")
- (nth 1 (car terms))))
- (progn
- (nconc eshell-sep-terms (list (nth 1 (car terms))))
- (setq subchains (cons (cdr sub-terms) subchains)
- sub-terms (list t)))
- (nconc sub-terms (list (car terms))))
- (setq terms (cdr terms)))
- (if (> (length sub-terms) 1)
- (setq subchains (cons (cdr sub-terms) subchains)))
- (if reversed
- (progn
- (if last-terms-sym
- (set last-terms-sym (reverse (cdr eshell-sep-terms))))
- subchains) ; already reversed
- (if last-terms-sym
- (set last-terms-sym (cdr eshell-sep-terms)))
- (nreverse subchains))))
+ (declare (obsolete eshell-split-commands "30.1"))
+ (let ((split-terms (eshell-split-commands terms separator reversed
+ last-terms-sym)))
+ (if last-terms-sym
+ (progn
+ (set last-terms-sym (cdr split-terms))
+ (car split-terms))
+ split-terms)))
;;_* Command evaluation macros
;;
--
2.25.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* bug#65162: 30.0.50; [PATCH] Simplify some internal Eshell command parsing functions
2023-08-09 0:16 bug#65162: 30.0.50; [PATCH] Simplify some internal Eshell command parsing functions Jim Porter
@ 2023-08-10 17:57 ` Jim Porter
0 siblings, 0 replies; 2+ messages in thread
From: Jim Porter @ 2023-08-10 17:57 UTC (permalink / raw)
To: 65162-done
On 8/8/2023 5:16 PM, Jim Porter wrote:
> These changes are just there for cleanup, and to make it easier to work
> with this code going forward. Since Eshell command parsing now has
> fairly-thorough regression tests, I think these changes should be pretty
> safe.
Merged to master as 60090abcbc5, and closing this now.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-08-10 17:57 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-09 0:16 bug#65162: 30.0.50; [PATCH] Simplify some internal Eshell command parsing functions Jim Porter
2023-08-10 17:57 ` Jim Porter
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).