From 5ff9194993ce9accab054bd73d68f4854947bcf0 Mon Sep 17 00:00:00 2001 From: Eshel Yaron Date: Sat, 4 Feb 2023 23:09:23 +0200 Subject: [PATCH] ENHANCED: handle renaming variables to existing variable names * sweeprolog.el (sweeprolog-rename-variable-allow-existing): new user option, used by... (sweeprolog-read-new-variable-try): new command, exists the minibuffer if it contains a valid variable name. It checks if the selected variable name already exists. Used by... (sweeprolog-read-new-variable): new function, reads a Prolog variable name in the minibuffer with a dedicated exit command. (sweeprolog-rename-variable): use it. --- README.org | 21 +++++++- sweeprolog.el | 139 ++++++++++++++++++++++++++++++++++++++------------ 2 files changed, 126 insertions(+), 34 deletions(-) diff --git a/README.org b/README.org index db2fd01..0dd0fe3 100644 --- a/README.org +++ b/README.org @@ -1650,7 +1650,7 @@ to provide contextual menus that you interact with using the mouse. - Command: context-menu-mode :: Toggle Context Menu mode. When enabled, clicking the mouse button ~down-mouse-3~ (i.e. right-click) - activates a menu whose contents depends on its surrounding context. + activates a menu whose contents depend on its surrounding context. - Variable: sweeprolog-context-menu-functions :: List of functions that create Context Menu entries for Prolog tokens. Each function should receive as its arguments the menu that is being created, the @@ -1669,7 +1669,9 @@ If you right-click on a Prolog file specification or module name, Sweep suggests visiting it either in the current window or in another. If you right-click on a predicate, it lets you view its documentation in a dedicated buffer (see also [[#prolog-help][Prolog Help]]). For variables, it -suggests to renaming (see . +enables the ~Rename Variable~ menu entry that you can use to rename the +variable you click on across its containing clause (see [[#rename-variable][Renaming +Variables]]). You can further extend and customize the context menu that ~sweeprolog-mode~ provides by adding functions to the variable @@ -1690,6 +1692,11 @@ following command: - Key: C-c C-r (sweeprolog-rename-variable) :: Rename a variable across the topmost Prolog term at point. +- User Option: sweeprolog-rename-variable-allow-existing :: If + non-nil, allow selecting an existing variable name as the new name + of a variable being renamed with ~sweeprolog-rename-variable~. If it + is the symbol ~confirm~, allow but ask for confirmation first. + Defaults to ~confirm~. The command ~sweeprolog-rename-variable~, bound to ~C-c C-r~, prompts for two variable names and replaces all occurrences of the first variable @@ -1697,6 +1704,16 @@ in the term at point with the second. The prompt for the first (old) variable name provides completion based on the existing variable names in the current term, and it uses the variable at point as its default. +The user option ~sweeprolog-rename-variable-allow-existing~ controls +what happens if the second (new) variable name that you insert in the +minibuffer already occurs in the current clause. By default it is set +to ~confirm~, which says to ask for confirmation before selecting an +existing variable name as the new name. This is because renaming a +variable to another existing variable name potentially alters the +semantics of the term by merging the two variables. Other +alternatives for this user option are ~t~ for allowing such merges +without confirmation, and ~nil~ for refusing them altogether. + If Context Menu mode is enabled, you can also rename variables by right-clicking on them with the mouse and selecting =Rename Variable= from the top of the context menu. See [[#context-menu][Context Menu]] for more diff --git a/sweeprolog.el b/sweeprolog.el index d594898..fcb4c85 100644 --- a/sweeprolog.el +++ b/sweeprolog.el @@ -387,6 +387,14 @@ token via its `help-echo' text property." :type 'boolean :group 'sweeprolog) +(defcustom sweeprolog-rename-variable-allow-existing 'confirm + "If non-nil, allow renaming variables to existing variable names. +If it is the symbol `confirm', allow but ask for confirmation +first." + :package-version '((sweeprolog "0.15.1")) + :type 'symbol + :group 'sweeprolog) + ;;;; Keymaps (defvar sweeprolog-mode-map @@ -5559,15 +5567,95 @@ is the name of the variable at point, if any." (defun sweeprolog--format-variable (var) (propertize var 'face (sweeprolog-variable-face))) -(defun sweeprolog-rename-variable (&optional old new point interactive) +(defvar sweeprolog-read-new-variable--existing-vars nil) +(defvar-local sweeprolog-read-new-variable--warned nil) + +(defun sweeprolog--variable-name-p (string) + "Return t if STRING is valid Prolog variable name." + (save-match-data + (let ((case-fold-search nil)) + (not + (not + (string-match (rx bos (or "_" upper) (* alnum) eos) string)))))) + +(defun sweeprolog-read-new-variable-try () + "Try to exit the minibuffer with a new Prolog variable name. + +If the minibuffer contains a variable that already occurs in the +current clause, warn and ask for confirmation before exiting. If +the minibuffer does not contain a valid variable name, report it +and refuse to exit." + (interactive) + (unless (> (minibuffer-depth) 0) + (error "Minibuffer must be active")) + (let ((choice (minibuffer-contents))) + (if (sweeprolog--variable-name-p choice) + (if (member choice sweeprolog-read-new-variable--existing-vars) + (pcase sweeprolog-rename-variable-allow-existing + ('confirm (if (string= sweeprolog-read-new-variable--warned choice) + (exit-minibuffer) + (minibuffer-message + (substitute-command-keys + "%s already exists, type \\[sweeprolog-read-new-variable-try] again to confirm") + (sweeprolog--format-variable choice)) + (setq sweeprolog-read-new-variable--warned choice))) + ('nil (minibuffer-message "%s already exists" (sweeprolog--format-variable choice))) + (_ (exit-minibuffer))) + (exit-minibuffer)) + (minibuffer-message "Invalid Prolog variable name")))) + +(put 'sweeprolog-read-new-variable-try :advertised-binding [?\C-m]) + +(defvar sweeprolog-read-new-variable-map + (let ((map (make-sparse-keymap))) + (set-keymap-parent map minibuffer-local-map) + (define-key map (kbd "C-m") #'sweeprolog-read-new-variable-try) + (define-key map (kbd "C-j") #'sweeprolog-read-new-variable-try) + map) + "Keymap used by `sweeprolog-read-new-variable'.") + +(defun sweeprolog-read-new-variable (prompt existing-vars) + "Prompt with PROMPT for a new Prolog variable. +EXISTING-VARS is a list of existing variable names (strings)." + (let ((sweeprolog-read-new-variable--existing-vars existing-vars)) + (read-from-minibuffer prompt nil sweeprolog-read-new-variable-map))) + +(defun sweeprolog-read-existing-variable (occurrences &optional default) + (let* ((max-var-len (apply #'max + (mapcar #'length + (mapcar #'car + occurrences)))) + (completion-extra-properties + (list :annotation-function + (lambda (key) + (let ((n (length (alist-get key + occurrences + nil nil + #'string=)))) + (concat (make-string (- max-var-len + (length key)) + ? ) + (format " %d %s" n + (ngettext "occurrence" + "occurrences" + n)))))))) + (completing-read + (concat + "Rename variable" + (when default + (concat " (default " (sweeprolog--format-variable default) ")")) + ": ") + occurrences nil t nil nil default))) + +(defun sweeprolog-rename-variable (&optional old new point verbose) "Rename the variable OLD to NEW in the Prolog term at POINT. If OLD is nil, prompt for it in the minibuffer with completion. If NEW is nil, prompt for it as well. If POINT is nil, it -defaults to the current point. If INTERACTIVE is non-nil, also -print a message with the number of replaced occurrences of OLD. +defaults to the current point. If VERBOSE is non-nil, also print +a message with the number of replaced occurrences of OLD. -Interactively, OLD, NEW and POINT are nil, and INTERACTIVE is t." +Interactively, OLD, NEW and POINT are nil, and VERBOSE is t." (interactive (list nil nil nil t) sweeprolog-mode) (setq point (or point (point))) (let* ((term-var-occurrences (sweeprolog--variables-at-point point)) @@ -5575,34 +5663,19 @@ Interactively, OLD, NEW and POINT are nil, and INTERACTIVE is t." (var-at-point (cdr term-var-occurrences))) (unless var-occurrences (user-error "No variables to rename here!")) - (let* ((max-var-len (apply #'max - (mapcar #'length - (mapcar #'car - var-occurrences)))) - (completion-extra-properties - (list :annotation-function - (lambda (key) - (let ((n (length (alist-get key var-occurrences nil nil #'string=)))) - (concat (make-string (- max-var-len (length key)) ? ) - (format " %d %s" n - (ngettext "occurrence" - "occurrences" - n))))))) - (old-name + (let* ((old-name (or old - (completing-read - (concat - "Rename variable" - (when-let ((def var-at-point)) - (concat " (default " (sweeprolog--format-variable def) ")")) - ": ") - var-occurrences nil t nil nil var-at-point))) + (sweeprolog-read-existing-variable var-occurrences + var-at-point))) + (old-formatted (sweeprolog--format-variable old-name)) + (existing-vars (mapcar #'car var-occurrences)) (new-name (or new - (read-string - (concat - "Rename " (sweeprolog--format-variable old-name) " to: ") - nil nil old-name))) + (sweeprolog-read-new-variable + (concat "Rename " + old-formatted + " to: ") + existing-vars))) (old-occurrences (alist-get old-name var-occurrences nil nil #'string=)) (num (length old-occurrences))) @@ -5614,11 +5687,13 @@ Interactively, OLD, NEW and POINT are nil, and INTERACTIVE is t." (delete-region occurrence-beg occurrence-end) (goto-char occurrence-beg) (insert new-name))))) - (when interactive - (message "Replaced %d %s of %s to %s." + (when verbose + (message (if (member new-name existing-vars) + "Merged %d %s of %s to %s." + "Replaced %d %s of %s to %s.") num (ngettext "occurrence" "occurrences" num) - (sweeprolog--format-variable old-name) + old-formatted (sweeprolog--format-variable new-name)))))) -- 2.39.2