* bug#36103: 24.5; Blank spaces around parentheses in history expansion in shell mode
2023-10-03 10:54 ` Mauro Aranda
@ 2023-10-07 13:39 ` Mauro Aranda
0 siblings, 0 replies; 4+ messages in thread
From: Mauro Aranda @ 2023-10-07 13:39 UTC (permalink / raw)
To: 36103; +Cc: Enric Rodríguez Carbonell, Lars Ingebrigtsen, Noam Postavsky
[-- Attachment #1: Type: text/plain, Size: 26 bytes --]
Here's a draft of my idea.
[-- Attachment #2: 0001-Give-comint-derived-modes-a-way-to-specialize-comint.patch --]
[-- Type: text/x-patch, Size: 9501 bytes --]
From 56628875e89b022ab42a2d78f207a4391a75198f Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Sun, 1 Oct 2023 09:51:53 -0300
Subject: [PATCH] Give comint derived modes a way to specialize
comint-arguments
The comint-arguments regexp approach for splitting arguments falls
short, as demonstrated in Bug#36103.
* lisp/comint.el (comint-arguments-function): New variable.
(comint-arguments-default-function): New function.
(comint-arguments): Use it.
(comint-delimiter-argument-list): Adapt docstring.
* lisp/shell.el (shell-arguments): New function. When treesitter
support is available for the shell program, specialize the way we
look for arguments in input.
(shell-mode): Set it as comint-arguments-function.
* test/lisp/shell-tests.el (shell-test-with-temporary-shell): New
macro.
(shell-test-history-expansion-helper): New function.
(shell-test-history-expansion): New test.
---
lisp/comint.el | 43 +++++++++++++++++++++++++++++----------
lisp/shell.el | 44 ++++++++++++++++++++++++++++++++++++++++
test/lisp/shell-tests.el | 35 ++++++++++++++++++++++++++++++++
3 files changed, 111 insertions(+), 11 deletions(-)
diff --git a/lisp/comint.el b/lisp/comint.el
index de7cc5b0e86..f0c05dd8e2c 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -202,13 +202,22 @@ comint-delimiter-argument-list
"List of characters to recognize as separate arguments in input.
Strings comprising a character in this list will separate the arguments
surrounding them, and also be regarded as arguments in their own right (unlike
-whitespace). See `comint-arguments'.
+whitespace). See `comint-arguments-default-function'.
Defaults to the empty list.
For shells, a good value is (?\\| ?& ?< ?> ?\\( ?\\) ?;).
This is a good thing to set in mode hooks.")
+(defvar-local comint-arguments-function #'comint-arguments-default-function
+ "Function to use to split arguments from an input.
+
+By default, this is `comint-arguments-default-function', which assumes, in
+general, that whitespace separates arguments and treats runs of characters in
+`comint-delimiter-argument-list' as a separate argument.
+
+This is a good thing to set in mode hooks.")
+
(defcustom comint-input-autoexpand nil
"If non-nil, expand input command history references on completion.
This mirrors the optional behavior of tcsh (its autoexpand and histlist).
@@ -1785,17 +1794,14 @@ comint-delim-arg
(setq args (cons (substring arg start pos) args))))
args)))
-(defun comint-arguments (string nth mth)
+(defun comint-arguments-default-function (string nth mth)
"Return from STRING the NTH to MTH arguments.
-NTH and/or MTH can be nil, which means the last argument.
-NTH and MTH can be negative to count from the end; -1 means
-the last argument.
-Returned arguments are separated by single spaces. We assume
-whitespace separates arguments, except within quotes and except
-for a space or tab that immediately follows a backslash. Also, a
-run of one or more of a single character in
-`comint-delimiter-argument-list' is a separate argument.
-Argument 0 is the command name."
+
+Used as the default function for `comint-arguments', returns the arguments
+separated by single spaces. Assumes that whitespace separates arguments,
+except within quotes and except for a space or tab that immediately follows a
+backslash. Also, a run of one or more of a single character in
+`comint-delimiter-argument-list' is a separate argument."
;; The first line handles ordinary characters and backslash-sequences
;; (except with w32 msdos-like shells, where backslashes are valid).
;; The second matches "-quoted strings.
@@ -1853,6 +1859,21 @@ comint-arguments
(t (1- (- mth))))))
(mapconcat
(lambda (a) a) (nthcdr n (nreverse (nthcdr m args))) " "))))
+
+(defun comint-arguments (string nth mth)
+ "Return from STRING the NTH to MTH arguments.
+
+NTH and/or MTH can be nil, which means the last argument.
+NTH and MTH can be negative to count from the end; -1 means
+the last argument.
+
+Argument 0 is the command name.
+
+Calls the `comint-arguments-function' with STRING, NTH and MTH as arguments
+and returns whatever that function returns, which should be the NTH to MTH
+arguments from STRING."
+ (funcall comint-arguments-function string nth mth))
+
\f
;;
;; Input processing stuff
diff --git a/lisp/shell.el b/lisp/shell.el
index 48978fecbdd..103f39bc455 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -100,6 +100,11 @@
(eval-when-compile (require 'files-x)) ;with-connection-local-variables
(require 'subr-x)
(eval-when-compile (require 'cl-lib))
+;; Used for better history expansion with event/word designators.
+(declare-function treesit-node-text "treesit.el")
+(declare-function treesit-node-on "treesit.el")
+(declare-function treesit-node-children "treesit.el")
+(declare-function treesit-ready-p "treesit.el")
;;; Customization and Buffer Variables
@@ -606,6 +611,44 @@ shell-completion-vars
(defvar sh-shell-file)
+(defun shell-arguments (string nth mth)
+ "Return from STRING the NTH to MTH arguments, separated by whitespace.
+
+Used as `comint-arguments-function'. When there's no tree-sitter support
+for the shell being used, falls back to `comint-arguments-default-function'."
+ (cond ((and (member shell--start-prog '("bash" "sh"))
+ (progn (require 'treesit)
+ (and (treesit-available-p)
+ (treesit-ready-p 'bash)))
+ (condition-case nil
+ (with-temp-buffer
+ (insert string)
+ (let ((inhibit-message t))
+ (bash-ts-mode))
+ (let* ((ts-node (treesit-node-on (point-min) (point-max)))
+ (args
+ (mapcar #'treesit-node-text
+ ;; We don't want to return ")" for
+ ;; a STRING like $( cat file ).
+ ;; So treat everything that's not a
+ ;; command node as a single node.
+ (if (string= (treesit-node-type ts-node)
+ "command")
+ (treesit-node-children ts-node)
+ (list ts-node))))
+ (count (length args))
+ (n (cond
+ ((null nth) (1- count))
+ ((>= nth 0) nth)
+ (t (+ count nth))))
+ (m (cond
+ ((null mth) count)
+ ((>= mth 0) (1+ mth))
+ (t (1+ (- count mth))))))
+ (mapconcat #'identity (seq-subseq args n m) " ")))
+ (error nil))))
+ (t (comint-arguments-default-function string nth mth))))
+
(define-derived-mode shell-mode comint-mode "Shell"
"Major mode for interacting with an inferior shell.
\\<shell-mode-map>
@@ -684,6 +727,7 @@ shell-mode
(setq-local shell-dirstack nil)
(setq-local shell-last-dir nil)
(setq-local comint-get-old-input #'shell-get-old-input)
+ (setq-local comint-arguments-function #'shell-arguments)
;; People expect Shell mode to keep the last line of output at
;; window bottom.
(setq-local scroll-conservatively 101)
diff --git a/test/lisp/shell-tests.el b/test/lisp/shell-tests.el
index ddddfdb2e0f..c88b18194d3 100644
--- a/test/lisp/shell-tests.el
+++ b/test/lisp/shell-tests.el
@@ -25,6 +25,7 @@
(require 'shell)
(require 'ert)
+(require 'treesit)
(ert-deftest shell-tests-unquote-1 ()
"Test problem found by Filipp Gunbin in emacs-devel."
@@ -95,4 +96,38 @@ shell-directory-tracker-cd
(should (not (equal start-dir list-buffers-directory)))
(should (string-prefix-p list-buffers-directory start-dir)))))
+(defmacro shell-test-with-temporary-shell (shell &rest body)
+ "Run a temporary SHELL and return the result of evaluating BODY."
+ (declare (indent defun))
+ `(let ((explicit-shell-file-name ,shell))
+ (shell)
+ (unwind-protect
+ (progn ,@body)
+ (let ((shell-kill-buffer-on-exit t))
+ (comint-send-eof)))))
+
+(defun shell-test-history-expansion-helper (cmd designator)
+ "Add CMD to comint history, expand DESIGNATOR and return its expansion."
+ (shell-test-with-temporary-shell "bash"
+ (comint-add-to-input-history cmd)
+ (end-of-buffer)
+ (let ((opoint (point)))
+ (insert designator)
+ (completion-at-point)
+ (prog1 (buffer-substring opoint (point))
+ (delete-region opoint (point))))))
+
+(ert-deftest shell-test-history-expansion ()
+ "Test that history expansion with designators works."
+ (skip-unless (and (treesit-ready-p 'bash)
+ (executable-find "bash")))
+ (let ((cmd "cat <( date )"))
+ ;; The following three tests come from Bug5007.
+ ;; TODO: More tests with different designators.
+ (should (string= (shell-test-history-expansion-helper cmd "!!") cmd))
+ (should (string= (shell-test-history-expansion-helper cmd "!:$") "<( date )"))
+ (should (string= (shell-test-history-expansion-helper
+ (substring cmd 4) "!:$")
+ (substring cmd 4)))))
+
;;; shell-tests.el ends here
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread