diff options
| -rw-r--r-- | gptel-tools/update_text_file.el | 330 | ||||
| -rw-r--r-- | modules/ai-config.el | 1 | ||||
| -rw-r--r-- | tests/test-update-text-file.el | 352 | ||||
| -rw-r--r-- | todo.org | 29 |
4 files changed, 576 insertions, 136 deletions
diff --git a/gptel-tools/update_text_file.el b/gptel-tools/update_text_file.el index 0125e2ab..492ed554 100644 --- a/gptel-tools/update_text_file.el +++ b/gptel-tools/update_text_file.el @@ -1,149 +1,231 @@ ;;; update_text_file.el --- Update text files for gptel -*- lexical-binding: t; -*- -;; Copyright (C) 2025 - -;; Author: gptel-tool-writer +;; Author: Craig Jennings <c@cjennings.net> ;; Keywords: convenience, tools ;; This file is not part of GNU Emacs. -;; This program is free software; you can redistribute it and/or modify -;; it under the terms of the GNU General Public License as published by -;; the Free Software Foundation, either version 3 of the License, or -;; (at your option) any later version. - -;; This program is distributed in the hope that it will be useful, -;; but WITHOUT ANY WARRANTY; without even the implied warranty of -;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -;; GNU General Public License for more details. - ;;; Commentary: -;; This file provides a gptel tool for updating text files with various -;; operations including replace, append, prepend, insert-at-line, and -;; delete-lines. The tool creates timestamped backups and shows diffs -;; before applying changes. +;; Gptel tool for updating an existing text file with one of five +;; operations: +;; +;; replace Replace all occurrences of PATTERN with REPLACEMENT. +;; append Add TEXT at the end of the file. +;; prepend Add TEXT at the beginning of the file. +;; insert-at-line Insert TEXT at LINE-NUM (1-indexed). +;; delete-lines Delete every line containing PATTERN. +;; +;; The operations are pure-string transforms — file I/O happens only at +;; the outer wrapper, which validates the path, takes a timestamped +;; backup, and writes the new content atomically. The tool uses gptel's +;; `:confirm t' meta-flag for the user-facing prompt, mirroring how +;; `write_text_file' handles confirmation. +;; +;; PATTERN is a literal substring for `replace' and `delete-lines'. No +;; regex. The model can build literal multi-line patterns and we don't +;; want it to discover regex metacharacter gotchas through trial and +;; error. ;;; Code: (require 'gptel) (require 'subr-x) +(require 'cl-lib) + +;; ---------------------------------------------------------------- helpers + +(defun cj/update-text-file--validate-path (path) + "Validate PATH for update. Return the truename on success. + +PATH must resolve inside the user's home directory, must exist, must +be a regular file, and must be readable and writable." + (let ((full (expand-file-name path "~"))) + (unless (string-prefix-p (expand-file-name "~") full) + (error "Path must be within home directory: %s" path)) + (unless (file-exists-p full) + (error "File not found: %s" full)) + (when (file-directory-p full) + (error "Path is a directory, not a file: %s" full)) + (unless (file-readable-p full) + (error "No read permission for file: %s" full)) + (unless (file-writable-p full) + (error "No write permission for file: %s" full)) + (if (file-symlink-p full) + (file-truename full) + full))) + +(defun cj/update-text-file--backup-name (path) + "Return a backup filename for PATH timestamped to the current second." + (format "%s-%s.bak" path (format-time-string "%Y-%m-%d-%H%M%S"))) + +(defconst cj/update-text-file--size-limit (* 10 1024 1024) + "Reject files larger than 10MB so a runaway operation can't churn the disk.") + +;; ----------------------------------------------------- string transforms +;; +;; Each transform takes the file contents as a string plus operation +;; parameters and returns the new contents. Pure functions — no I/O. + +(defun cj/update-text-file--replace (content pattern replacement) + "Return CONTENT with every occurrence of PATTERN replaced by REPLACEMENT. +PATTERN is treated as a literal substring. Signal an error if PATTERN is +empty or nil." + (unless (and (stringp pattern) (> (length pattern) 0)) + (error "Replace operation requires a non-empty pattern")) + (unless (stringp replacement) + (error "Replace operation requires a replacement string")) + (replace-regexp-in-string (regexp-quote pattern) replacement content t t)) + +(defun cj/update-text-file--append (content text) + "Return CONTENT with TEXT added at the end, separated by a newline. +A trailing newline is guaranteed. Signal if TEXT is nil or empty." + (unless (and (stringp text) (> (length text) 0)) + (error "Append operation requires non-empty text")) + (let ((base (if (or (string-empty-p content) + (string-suffix-p "\n" content)) + content + (concat content "\n")))) + (if (string-suffix-p "\n" text) + (concat base text) + (concat base text "\n")))) -;; Helper function for building sed commands -(defun cj/build-sed-command (operation pattern replacement line-num temp-file) - "Build appropriate sed/shell command for OPERATION." +(defun cj/update-text-file--prepend (content text) + "Return CONTENT with TEXT added at the beginning. +TEXT is separated from CONTENT by a newline. Signal if TEXT is nil +or empty." + (unless (and (stringp text) (> (length text) 0)) + (error "Prepend operation requires non-empty text")) + (if (string-suffix-p "\n" text) + (concat text content) + (concat text "\n" content))) + +(defun cj/update-text-file--insert-at-line (content line-num text) + "Return CONTENT with TEXT inserted before LINE-NUM (1-indexed). +LINE-NUM 1 prepends. LINE-NUM one past the last line appends. Signal +on out-of-range LINE-NUM or empty TEXT." + (unless (and (integerp line-num) (> line-num 0)) + (error "Insert-at-line requires a positive integer line number")) + (unless (and (stringp text) (> (length text) 0)) + (error "Insert-at-line requires non-empty text")) + (let* ((lines (split-string content "\n")) + ;; `split-string' on a newline-terminated string returns an + ;; extra empty element at the end. Trim it so the line count + ;; matches what a human would say. + (trailing-newline (string-suffix-p "\n" content)) + (line-count (if trailing-newline + (1- (length lines)) + (length lines)))) + (when (> line-num (1+ line-count)) + (error "Line %d out of range (file has %d lines)" line-num line-count)) + (let* ((to-insert (if (string-suffix-p "\n" text) + (substring text 0 (1- (length text))) + text)) + (idx (1- line-num)) + (head (cl-subseq lines 0 idx)) + (tail (cl-subseq lines idx))) + (mapconcat #'identity + (append head (list to-insert) tail) + "\n")))) + +(defun cj/update-text-file--delete-lines (content pattern) + "Return CONTENT with every line containing PATTERN removed. +PATTERN is a literal substring. Trailing-newline state is preserved +when at least one line survives; an empty result is returned as the +empty string." + (unless (and (stringp pattern) (> (length pattern) 0)) + (error "Delete-lines requires a non-empty pattern")) + (let* ((trailing-newline (string-suffix-p "\n" content)) + (raw-lines (split-string content "\n")) + ;; Drop the trailing empty element split-string produces when + ;; the input ends in a newline. + (lines (if trailing-newline + (butlast raw-lines) + raw-lines)) + (kept (cl-remove-if (lambda (line) + (string-match-p (regexp-quote pattern) line)) + lines))) + (cond + ((null kept) "") + (trailing-newline (concat (mapconcat #'identity kept "\n") "\n")) + (t (mapconcat #'identity kept "\n"))))) + +(defun cj/update-text-file--apply-operation + (content operation pattern replacement line-num) + "Dispatch OPERATION on CONTENT. Return the transformed string. + +OPERATION is one of \"replace\", \"append\", \"prepend\", +\"insert-at-line\", or \"delete-lines\". PATTERN, REPLACEMENT, and +LINE-NUM are used per operation; unused arguments are ignored." (pcase operation - ("replace" - (unless (and pattern replacement) - (error "Replace operation requires pattern and replacement")) - (format "sed -i 's|%s|%s|g' '%s'" - (replace-regexp-in-string "|" "\\\\\\\\|" pattern) - (replace-regexp-in-string "|" "\\\\\\\\|" replacement) - temp-file)) - ("append" - (unless pattern - (error "Append operation requires text to append")) - (format "printf '%%s\\\\n' %s >> '%s'" - (shell-quote-argument pattern) - temp-file)) - ("prepend" - (unless pattern - (error "Prepend operation requires text to prepend")) - (format "(printf '%%s\\\\n' %s; cat '%s') > '%s.new' && mv '%s.new' '%s'" - (shell-quote-argument pattern) - temp-file temp-file temp-file temp-file)) - ("insert-at-line" - (unless (and pattern line-num) - (error "Insert-at-line requires text and line number")) - (format "sed -i '%di\\\\%s' '%s'" - line-num - (replace-regexp-in-string "/" "\\\\\\\\/" pattern) - temp-file)) - ("delete-lines" - (unless pattern - (error "Delete-lines requires pattern")) - (format "sed -i '/%s/d' '%s'" - (replace-regexp-in-string "/" "\\\\\\\\/" pattern) - temp-file)) - (_ - (error "Unknown operation: %s" operation)))) - -;; Main tool definition + ("replace" (cj/update-text-file--replace content pattern replacement)) + ("append" (cj/update-text-file--append content pattern)) + ("prepend" (cj/update-text-file--prepend content pattern)) + ("insert-at-line" (cj/update-text-file--insert-at-line content line-num pattern)) + ("delete-lines" (cj/update-text-file--delete-lines content pattern)) + (_ (error "Unknown operation: %s" operation)))) + +;; ----------------------------------------------------- file-level wrapper + +(defun cj/update-text-file--run (path operation pattern replacement line-num) + "Update PATH with OPERATION and return a status string. + +PATTERN, REPLACEMENT, and LINE-NUM are passed through per operation. +A timestamped backup is created next to the file before writing. If +the operation produces no change the backup is removed and the file +is left untouched." + (let* ((full (cj/update-text-file--validate-path path)) + (size (file-attribute-size (file-attributes full)))) + (when (> size cj/update-text-file--size-limit) + (error "File too large (%s): exceeds 10MB limit" + (file-size-human-readable size))) + (let* ((before (with-temp-buffer + (insert-file-contents full) + (buffer-string))) + (after (cj/update-text-file--apply-operation + before operation pattern replacement line-num))) + (cond + ((string= before after) + (format "No changes made to %s" full)) + (t + (let ((backup (cj/update-text-file--backup-name full))) + (copy-file full backup t) + (with-temp-file full (insert after)) + (format "Updated %s (backup: %s)" + full (file-name-nondirectory backup)))))))) + +;; ----------------------------------------------------- tool registration + (with-eval-after-load 'gptel (gptel-make-tool :name "update_text_file" :function (lambda (path operation &optional pattern replacement line-num) - (let* ((full-path (expand-file-name path "~")) - (temp-file (make-temp-file "gptel-update-" nil ".tmp")) - (backup-name (format "%s-%s.bak" - full-path - (format-time-string "%Y-%m-%d-%H%M%S")))) - (unwind-protect - (progn - ;; Validate path - (unless (string-prefix-p (expand-file-name "~") full-path) - (error "Path must be within home directory")) - (unless (file-exists-p full-path) - (error "File not found: %s" full-path)) - (unless (file-readable-p full-path) - (error "No read permission for file: %s" full-path)) - ;; Check file size - (let ((size (file-attribute-size (file-attributes full-path)))) - (when (> size (* 10 1024 1024)) - (error "File too large (%s): exceeds 10MB limit" - (file-size-human-readable size)))) - ;; Create backup - (copy-file full-path backup-name t) - ;; Copy to temp file for operations - (copy-file full-path temp-file t) - ;; Execute operation and check diff - (let* ((sed-cmd (cj/build-sed-command operation pattern replacement line-num temp-file)) - (result (shell-command-to-string sed-cmd)) - (diff-output (shell-command-to-string - (format "diff -u '%s' '%s' 2>/dev/null" full-path temp-file)))) - (if (string-empty-p diff-output) - (progn - (delete-file backup-name) - (format "No changes made to %s" full-path)) - (if (y-or-n-p (format "Apply these changes to %s?\\n\\n%s\\n" - full-path diff-output)) - (progn - (copy-file temp-file full-path t) - (format "Updated %s (backup: %s)" - full-path (file-name-nondirectory backup-name))) - (progn - (delete-file backup-name) - (error "Update cancelled by user")))))) - ;; Cleanup temp file - (when (file-exists-p temp-file) - (delete-file temp-file))))) - :description "Update a text file with various operations: replace, append, prepend, insert-at-line, or delete-lines. Shows diff before applying changes and creates timestamped backups." + (cj/update-text-file--run path operation pattern replacement line-num)) + :description "Update an existing text file with one of: replace, append, prepend, insert-at-line, delete-lines. Creates a timestamped backup before writing. Patterns are literal substrings, not regex." :args (list '(:name "path" - :type string - :description "File path relative to home directory, e.g., 'documents/myfile.txt' or '~/documents/myfile.txt'") - '(:name "operation" - :type string - :enum ["replace" "append" "prepend" "insert-at-line" "delete-lines"] - :description "The type of update operation to perform") - '(:name "pattern" - :type string - :description "For replace/delete: pattern to match. For append/prepend/insert: text to add" - :optional t) - '(:name "replacement" - :type string - :description "For replace operation: the replacement text" - :optional t) - '(:name "line_num" - :type integer - :description "For insert-at-line operation: the line number where to insert" - :optional t)) + :type string + :description "File path relative to home directory, e.g. 'documents/foo.txt' or '~/documents/foo.txt'") + '(:name "operation" + :type string + :enum ["replace" "append" "prepend" "insert-at-line" "delete-lines"] + :description "Which update operation to perform") + '(:name "pattern" + :type string + :description "For replace/delete-lines: the literal substring to match. For append/prepend/insert-at-line: the text to add. Required for every operation." + :optional t) + '(:name "replacement" + :type string + :description "For replace: the literal replacement text. Ignored by other operations." + :optional t) + '(:name "line_num" + :type integer + :description "For insert-at-line: 1-indexed line number to insert before. Ignored by other operations." + :optional t)) :category "filesystem" :confirm t - :include t)) - -;; Automatically add to gptel-tools on load -(add-to-list 'gptel-tools (gptel-get-tool '("filesystem" "update_text_file"))) + :include t) + (add-to-list 'gptel-tools (gptel-get-tool '("filesystem" "update_text_file")))) (provide 'update_text_file) -;;; update_text_file.el ends here" +;;; update_text_file.el ends here diff --git a/modules/ai-config.el b/modules/ai-config.el index 2783bc18..bc896704 100644 --- a/modules/ai-config.el +++ b/modules/ai-config.el @@ -52,6 +52,7 @@ '(read_buffer read_text_file write_text_file + update_text_file list_directory_files move_to_trash) "Feature symbols for optional local GPTel tool modules." diff --git a/tests/test-update-text-file.el b/tests/test-update-text-file.el new file mode 100644 index 00000000..d689274a --- /dev/null +++ b/tests/test-update-text-file.el @@ -0,0 +1,352 @@ +;;; test-update-text-file.el --- Tests for update_text_file gptel tool -*- lexical-binding: t; -*- + +;;; Commentary: +;; Normal / Boundary / Error tests for each operation in +;; gptel-tools/update_text_file.el, plus file-level wrapper tests. +;; The pure-string helpers carry most of the coverage; the wrapper +;; only adds the I/O surface (backup, write, validation). + +;;; Code: + +(require 'ert) +(require 'cl-lib) + +(eval-and-compile + (add-to-list 'load-path (expand-file-name "tests" user-emacs-directory)) + (add-to-list 'load-path (expand-file-name "gptel-tools" user-emacs-directory)) + (setq load-prefer-newer t) + ;; Stub gptel so the tool file can be loaded without the real package. + (unless (featurep 'gptel) + (defvar gptel-tools nil) + (defun gptel-make-tool (&rest _args) nil) + (defun gptel-get-tool (&rest _args) nil) + (provide 'gptel))) + +(require 'update_text_file) + +;; ----------------------------------------------------- helpers + +(defun test-update-text-file--with-temp (content fn) + "Write CONTENT to a temp file, call FN with its path, then delete." + (let ((path (make-temp-file "test-update-text-file-"))) + (unwind-protect + (progn + (with-temp-file path (insert content)) + (funcall fn path)) + (when (file-exists-p path) (delete-file path))))) + +;; ----------------------------------------------------- replace + +(ert-deftest test-update-text-file-replace-normal () + "Normal: replace all occurrences of the literal pattern." + (should (equal (cj/update-text-file--replace "foo bar foo" "foo" "BAZ") + "BAZ bar BAZ"))) + +(ert-deftest test-update-text-file-replace-boundary-no-match () + "Boundary: pattern absent returns content unchanged." + (should (equal (cj/update-text-file--replace "abc" "xyz" "QQ") "abc"))) + +(ert-deftest test-update-text-file-replace-boundary-special-chars () + "Boundary: regex metacharacters in pattern are treated as literals." + (should (equal (cj/update-text-file--replace "a.b.c" "." "-") "a-b-c")) + (should (equal (cj/update-text-file--replace "(x)(y)" "(x)" "_") "_(y)")) + (should (equal (cj/update-text-file--replace "a$b" "$" "S") "aSb"))) + +(ert-deftest test-update-text-file-replace-boundary-unicode () + "Boundary: unicode in both pattern and replacement." + (should (equal (cj/update-text-file--replace "café résumé" "café" "thé") + "thé résumé"))) + +(ert-deftest test-update-text-file-replace-boundary-replacement-with-backref-like () + "Boundary: replacement strings with \\1 etc. are literal, not back-refs." + (should (equal (cj/update-text-file--replace "foo" "foo" "\\1bar") + "\\1bar"))) + +(ert-deftest test-update-text-file-replace-error-empty-pattern () + "Error: empty pattern signals." + (should-error (cj/update-text-file--replace "abc" "" "x"))) + +(ert-deftest test-update-text-file-replace-error-nil-pattern () + "Error: nil pattern signals." + (should-error (cj/update-text-file--replace "abc" nil "x"))) + +(ert-deftest test-update-text-file-replace-error-nil-replacement () + "Error: nil replacement signals." + (should-error (cj/update-text-file--replace "abc" "a" nil))) + +;; ----------------------------------------------------- append + +(ert-deftest test-update-text-file-append-normal () + "Normal: append adds text plus a trailing newline." + (should (equal (cj/update-text-file--append "line1\n" "line2") + "line1\nline2\n"))) + +(ert-deftest test-update-text-file-append-boundary-no-trailing-newline () + "Boundary: appends still produce a newline when content has none." + (should (equal (cj/update-text-file--append "abc" "def") + "abc\ndef\n"))) + +(ert-deftest test-update-text-file-append-boundary-empty-content () + "Boundary: appending to empty content yields just the new text + newline." + (should (equal (cj/update-text-file--append "" "hello") "hello\n"))) + +(ert-deftest test-update-text-file-append-boundary-text-with-trailing-newline () + "Boundary: text that already ends in newline isn't duplicated." + (should (equal (cj/update-text-file--append "a\n" "b\n") "a\nb\n"))) + +(ert-deftest test-update-text-file-append-error-empty-text () + "Error: empty text signals." + (should-error (cj/update-text-file--append "foo" ""))) + +(ert-deftest test-update-text-file-append-error-nil-text () + "Error: nil text signals." + (should-error (cj/update-text-file--append "foo" nil))) + +;; ----------------------------------------------------- prepend + +(ert-deftest test-update-text-file-prepend-normal () + "Normal: prepend adds text plus a separator newline." + (should (equal (cj/update-text-file--prepend "line1\n" "line0") + "line0\nline1\n"))) + +(ert-deftest test-update-text-file-prepend-boundary-empty-content () + "Boundary: prepending to empty content keeps just the new text + sep." + (should (equal (cj/update-text-file--prepend "" "hello") "hello\n"))) + +(ert-deftest test-update-text-file-prepend-boundary-text-with-trailing-newline () + "Boundary: text already terminated by newline is not double-broken." + (should (equal (cj/update-text-file--prepend "rest" "first\n") + "first\nrest"))) + +(ert-deftest test-update-text-file-prepend-error-empty-text () + "Error: empty text signals." + (should-error (cj/update-text-file--prepend "foo" ""))) + +(ert-deftest test-update-text-file-prepend-error-nil-text () + "Error: nil text signals." + (should-error (cj/update-text-file--prepend "foo" nil))) + +;; ----------------------------------------------------- insert-at-line + +(ert-deftest test-update-text-file-insert-at-line-normal () + "Normal: insert before line 2 of a 3-line file." + (should (equal (cj/update-text-file--insert-at-line "a\nb\nc\n" 2 "X") + "a\nX\nb\nc\n"))) + +(ert-deftest test-update-text-file-insert-at-line-boundary-first-line () + "Boundary: inserting at line 1 prepends." + (should (equal (cj/update-text-file--insert-at-line "a\nb\n" 1 "X") + "X\na\nb\n"))) + +(ert-deftest test-update-text-file-insert-at-line-boundary-one-past-end () + "Boundary: inserting one past the last line appends." + (should (equal (cj/update-text-file--insert-at-line "a\nb\n" 3 "X") + "a\nb\nX\n"))) + +(ert-deftest test-update-text-file-insert-at-line-boundary-no-trailing-newline () + "Boundary: works on content without a trailing newline." + (should (equal (cj/update-text-file--insert-at-line "a\nb" 2 "X") + "a\nX\nb"))) + +(ert-deftest test-update-text-file-insert-at-line-error-out-of-range () + "Error: line number beyond file length signals." + (should-error (cj/update-text-file--insert-at-line "a\nb\n" 5 "X"))) + +(ert-deftest test-update-text-file-insert-at-line-error-zero () + "Error: line number 0 signals." + (should-error (cj/update-text-file--insert-at-line "a\n" 0 "X"))) + +(ert-deftest test-update-text-file-insert-at-line-error-negative () + "Error: negative line number signals." + (should-error (cj/update-text-file--insert-at-line "a\n" -1 "X"))) + +(ert-deftest test-update-text-file-insert-at-line-error-empty-text () + "Error: empty text signals." + (should-error (cj/update-text-file--insert-at-line "a\n" 1 ""))) + +;; ----------------------------------------------------- delete-lines + +(ert-deftest test-update-text-file-delete-lines-normal () + "Normal: removes lines containing the literal pattern." + (should (equal (cj/update-text-file--delete-lines "keep\nkill me\nkeep\n" "kill") + "keep\nkeep\n"))) + +(ert-deftest test-update-text-file-delete-lines-boundary-no-match () + "Boundary: pattern matches nothing returns content unchanged." + (should (equal (cj/update-text-file--delete-lines "a\nb\nc\n" "z") + "a\nb\nc\n"))) + +(ert-deftest test-update-text-file-delete-lines-boundary-all-lines-match () + "Boundary: every line removed yields the empty string." + (should (equal (cj/update-text-file--delete-lines "x\nx\nx\n" "x") ""))) + +(ert-deftest test-update-text-file-delete-lines-boundary-special-chars-literal () + "Boundary: regex metacharacters in pattern are treated as literals." + (should (equal (cj/update-text-file--delete-lines "a.b\naxb\n" ".") + "axb\n"))) + +(ert-deftest test-update-text-file-delete-lines-boundary-no-trailing-newline () + "Boundary: content without trailing newline keeps that shape." + (should (equal (cj/update-text-file--delete-lines "keep\ndrop" "drop") + "keep"))) + +(ert-deftest test-update-text-file-delete-lines-error-empty-pattern () + "Error: empty pattern signals." + (should-error (cj/update-text-file--delete-lines "a\nb\n" ""))) + +(ert-deftest test-update-text-file-delete-lines-error-nil-pattern () + "Error: nil pattern signals." + (should-error (cj/update-text-file--delete-lines "a\nb\n" nil))) + +;; ----------------------------------------------------- apply-operation + +(ert-deftest test-update-text-file-apply-operation-dispatch () + "Each operation name dispatches to its transform." + (should (equal (cj/update-text-file--apply-operation "abc" "replace" "b" "B" nil) + "aBc")) + (should (equal (cj/update-text-file--apply-operation "a" "append" "b" nil nil) + "a\nb\n")) + (should (equal (cj/update-text-file--apply-operation "a" "prepend" "b" nil nil) + "b\na")) + (should (equal (cj/update-text-file--apply-operation "a\nb\n" "insert-at-line" "X" nil 2) + "a\nX\nb\n")) + (should (equal (cj/update-text-file--apply-operation "a\nb\n" "delete-lines" "a" nil nil) + "b\n"))) + +(ert-deftest test-update-text-file-apply-operation-error-unknown () + "Unknown operation signals." + (should-error (cj/update-text-file--apply-operation "x" "frobnicate" nil nil nil))) + +;; ----------------------------------------------------- validate-path + +(ert-deftest test-update-text-file-validate-path-normal () + "Normal: an existing readable+writable file under HOME passes." + (let* ((file (make-temp-file "test-update-text-file-"))) + (unwind-protect + (progn + ;; make-temp-file may land in /tmp; rebase to HOME for the test. + (let* ((home-file (expand-file-name + (concat ".test-update-text-file-" (format-time-string "%s") ".tmp") + "~"))) + (unwind-protect + (progn + (copy-file file home-file t) + (should (equal (cj/update-text-file--validate-path home-file) + (file-truename home-file)))) + (when (file-exists-p home-file) (delete-file home-file))))) + (when (file-exists-p file) (delete-file file))))) + +(ert-deftest test-update-text-file-validate-path-error-missing () + "Error: a missing file under HOME signals." + (let ((path (expand-file-name + (concat ".test-update-text-file-missing-" + (format-time-string "%s") ".tmp") + "~"))) + (when (file-exists-p path) (delete-file path)) + (should-error (cj/update-text-file--validate-path path)))) + +(ert-deftest test-update-text-file-validate-path-error-outside-home () + "Error: a path outside HOME signals." + (should-error (cj/update-text-file--validate-path "/etc/hostname"))) + +(ert-deftest test-update-text-file-validate-path-error-directory () + "Error: a directory signals." + (should-error (cj/update-text-file--validate-path "~"))) + +;; ----------------------------------------------------- backup-name + +(ert-deftest test-update-text-file-backup-name-shape () + "Backup names append a timestamped .bak suffix." + (let ((name (cj/update-text-file--backup-name "/home/user/foo.txt"))) + (should (string-prefix-p "/home/user/foo.txt-" name)) + (should (string-suffix-p ".bak" name)) + ;; Format is YYYY-MM-DD-HHMMSS. + (should (string-match-p "-[0-9]\\{4\\}-[0-9]\\{2\\}-[0-9]\\{2\\}-[0-9]\\{6\\}\\.bak\\'" + name)))) + +;; ----------------------------------------------------- file-level wrapper + +(defun test-update-text-file--in-home (suffix content fn) + "Write CONTENT to a temp file under HOME with SUFFIX, call FN, then delete. +Backups (path-TS.bak) are cleaned up after FN returns." + (let* ((name (format ".test-update-text-file-%s-%s.tmp" + suffix (format-time-string "%s%N"))) + (path (expand-file-name name "~"))) + (unwind-protect + (progn + (with-temp-file path (insert content)) + (funcall fn path)) + (when (file-exists-p path) (delete-file path)) + (dolist (b (file-expand-wildcards (concat path "-*.bak"))) + (when (file-exists-p b) (delete-file b)))))) + +(ert-deftest test-update-text-file-run-replace-normal () + "Wrapper: replace operation rewrites the file and creates a backup." + (test-update-text-file--in-home + "replace" "alpha bravo alpha\n" + (lambda (path) + (let ((result (cj/update-text-file--run path "replace" "alpha" "GAMMA" nil))) + (should (string-match-p "Updated" result)) + (should (string-match-p "backup:" result)) + (with-temp-buffer + (insert-file-contents path) + (should (equal (buffer-string) "GAMMA bravo GAMMA\n"))) + (should (file-expand-wildcards (concat path "-*.bak"))))))) + +(ert-deftest test-update-text-file-run-no-change-no-backup () + "Wrapper: no-op operation leaves the file untouched and creates no backup." + (test-update-text-file--in-home + "noop" "abc\n" + (lambda (path) + (let ((result (cj/update-text-file--run path "replace" "zzz" "QQ" nil))) + (should (string-match-p "No changes" result)) + (with-temp-buffer + (insert-file-contents path) + (should (equal (buffer-string) "abc\n"))) + (should-not (file-expand-wildcards (concat path "-*.bak"))))))) + +(ert-deftest test-update-text-file-run-append-normal () + "Wrapper: append operation adds a line to the file." + (test-update-text-file--in-home + "append" "first\n" + (lambda (path) + (cj/update-text-file--run path "append" "second" nil nil) + (with-temp-buffer + (insert-file-contents path) + (should (equal (buffer-string) "first\nsecond\n")))))) + +(ert-deftest test-update-text-file-run-insert-at-line-normal () + "Wrapper: insert-at-line inserts and rewrites the file." + (test-update-text-file--in-home + "insert" "a\nb\nc\n" + (lambda (path) + (cj/update-text-file--run path "insert-at-line" "X" nil 2) + (with-temp-buffer + (insert-file-contents path) + (should (equal (buffer-string) "a\nX\nb\nc\n")))))) + +(ert-deftest test-update-text-file-run-delete-lines-normal () + "Wrapper: delete-lines removes matching lines." + (test-update-text-file--in-home + "delete" "keep1\nkill\nkeep2\nkill\n" + (lambda (path) + (cj/update-text-file--run path "delete-lines" "kill" nil nil) + (with-temp-buffer + (insert-file-contents path) + (should (equal (buffer-string) "keep1\nkeep2\n")))))) + +(ert-deftest test-update-text-file-run-error-missing-file () + "Wrapper: missing file signals." + (let ((path (expand-file-name + (concat ".test-update-text-file-absent-" + (format-time-string "%s") ".tmp") + "~"))) + (when (file-exists-p path) (delete-file path)) + (should-error (cj/update-text-file--run path "append" "x" nil nil)))) + +(ert-deftest test-update-text-file-run-error-outside-home () + "Wrapper: path outside home signals." + (should-error (cj/update-text-file--run "/etc/hostname" "append" "x" nil nil))) + +(provide 'test-update-text-file) +;;; test-update-text-file.el ends here @@ -2583,18 +2583,23 @@ In scope: Out of scope: the F9 =ai-vterm= Claude-Code launcher (=modules/ai-vterm.el=) — separate module, working well. -*** TODO [#B] Wire update_text_file into cj/gptel-local-tool-features, then harden it :refactor:tests: - -The tool exists at =gptel-tools/update_text_file.el= (replace / append / prepend / insert-at-line / delete-lines, with diff preview and timestamped backups), but it's incomplete and needs hardening before it goes live in =cj/gptel-local-tool-features=. - -Scope: - -1. *Read the tool end-to-end* against the gptel-tools convention used by =read_buffer.el=, =write_text_file.el=, =move_to_trash.el=, and the other already-wired tools. Note where =update_text_file.el= diverges in shape (argument naming, error handling, return-value structure, backup placement). -2. *Refactor for completeness and consistency.* Bring it in line with the other tools: same argument-validation style, same diff-preview surface, same backup directory and naming, same error categories. Extract pure helpers where the current shape forces wrapper-heavy testing. -3. *Add ERT coverage targeting ~100%.* Normal / Boundary / Error per the standard project discipline: each verb (=replace=, =append=, =prepend=, =insert-at-line=, =delete-lines=) gets at least one Normal, one Boundary (empty file, single line, EOF, unicode), and one Error (missing file, line-range out of range, permission denied, write-conflict). Diff preview and backup creation get their own focused tests. -4. *Wire it into =cj/gptel-local-tool-features=* in =modules/ai-config.el= -- one-line addition to the defcustom default -- only once 1-3 are green. - -Acceptance: after restart, =gptel-tools= (the transient context view) shows =update_text_file= alongside =read_buffer=, =read_text_file=, =write_text_file=, =list_directory_files=, =move_to_trash=. =make test-file FILE=test-update-text-file.el= exits 0. =make coverage= reports =gptel-tools/update_text_file.el= at ≥95%. +*** 2026-05-16 Sat @ 01:17:58 -0500 Rewrote update_text_file.el and wired it into cj/gptel-local-tool-features + +I rewrote =gptel-tools/update_text_file.el= in pure Elisp. The previous +version shelled out to sed for everything, had a stray quote terminator +at EOF, produced literal backslash-n where actual newlines were +expected, and prompted via =y-or-n-p= redundantly with gptel's own +=:confirm t= flag. + +The five operations (=replace=, =append=, =prepend=, =insert-at-line=, +=delete-lines=) split into pure string transforms that test without +touching the disk. The file-level wrapper validates the path, enforces +the 10MB size limit, takes a timestamped backup, and writes atomically. +No backup is created when the operation is a no-op. + +=tests/test-update-text-file.el= covers Normal / Boundary / Error per +operation plus the wrapper -- 48 tests green. Added =update_text_file= +to =cj/gptel-local-tool-features= so gptel exposes it on next restart. *** TODO [#B] Fix gptel-magit triggers :bug: |