From 0000dae69fbbe55988eef2d3b179340fa8c6a7cc Mon Sep 17 00:00:00 2001 From: Vineet Kumar Date: Mon, 25 May 2026 15:09:58 -0400 Subject: make mostly working hashline implementation, just needs more tests --- llm-tools-hl-test.el | 180 ++++++++++++++++++++++++++++++++++++++++++++++++++- llm-tools-hl.el | 168 +++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 334 insertions(+), 14 deletions(-) diff --git a/llm-tools-hl-test.el b/llm-tools-hl-test.el index 4bc0fa0..85777c8 100644 --- a/llm-tools-hl-test.el +++ b/llm-tools-hl-test.el @@ -2,7 +2,7 @@ (require 'llm-tools-hl) (ert-deftest llm-tools--hl-file-read-test () - "Test that 'llm-tools--hl-file-read' produces correct hashline format." + "Test that 'llm-tools--hl-file-read' produces correct hashline format." (let* ((content "\ const TITLE = \"Mr\"; export function greet(name) { @@ -23,3 +23,181 @@ export function greet(name) { (unwind-protect (should (equal actual expected)) (delete-file file)))) + +(ert-deftest llm-tools--hl-anchor-to-line-num-test () + (should (= (llm-tools--hl-anchor-to-line-num "5ff") 5)) + (should (= (llm-tools--hl-anchor-to-line-num "123ab") 123)) + (should (= (llm-tools--hl-anchor-to-line-num "1vx") 1))) + +(ert-deftest llm-tools--hl-parse-section-single-replace () + (let ((section "= 1vx..1vx\n~const TITLE = \"Mrs\";")) + (should (= (length (llm-tools--hl-parse-section section)) 1)) + (let ((op (car (llm-tools--hl-parse-section section)))) + (should (eq (hl-op-type op) 'replace)) + (should (equal (hl-op-range op) '("1vx" "1vx"))) + (should (equal (hl-op-payload op) '("const TITLE = \"Mrs\";")))))) + +(ert-deftest llm-tools--hl-parse-section-multi-line-payload () + (let ((section "= 3gv..6be\n~\treturn [\n~\t\t\"Mrs\",\n~\t\tname?.trim() || \"guest\",\n~\t].join(\" \");")) + (let ((op (car (llm-tools--hl-parse-section section)))) + (should (= (length (hl-op-payload op)) 4))))) + +(ert-deftest llm-tools--hl-parse-section-blank-replace () + "Replace with no payload should produce nil payload, not empty list." + (let ((section "= 5ff..5ff")) + (let ((op (car (llm-tools--hl-parse-section section)))) + (should (eq (hl-op-type op) 'replace)) + (should (null (hl-op-payload op)))))) + +(ert-deftest llm-tools--hl-parse-section-insert-after () + (let ((section "+ 4ei\n~\t\t\"Dr\",")) + (let ((op (car (llm-tools--hl-parse-section section)))) + (should (eq (hl-op-type op) 'insert-after)) + (should (string= (hl-op-anchor op) "4ei")) + (should (equal (hl-op-payload op) '("\t\t\"Dr\",")))))) + +(ert-deftest llm-tools--hl-parse-section-insert-before () + (let ((section "< 5ff\n~\t\t\"Dr\",")) + (let ((op (car (llm-tools--hl-parse-section section)))) + (should (eq (hl-op-type op) 'insert-before)) + (should (string= (hl-op-anchor op) "5ff"))))) + +(ert-deftest llm-tools--hl-parse-section-delete () + (let ((section "- 5ff..5ff")) + (let ((op (car (llm-tools--hl-parse-section section)))) + (should (eq (hl-op-type op) 'delete)) + (should (equal (hl-op-range op) '("5ff" "5ff")))))) + +(ert-deftest llm-tools--hl-parse-section-skip-comments-and-blanks () + (let ((section "# comment\n\n+ 4ei\n~line")) + (should (= (length (llm-tools--hl-parse-section section)) 1)))) + +(ert-deftest llm-tools--hl-parse-section-multiple-ops () + "Two separate insert ops in one section." + (let ((section "+ 4ei\n~first\n< 5ff\n~second")) + (let ((ops (llm-tools--hl-parse-section section))) + (should (= (length ops) 2)) + (should (eq (hl-op-type (car ops)) 'insert-after)) + (should (eq (hl-op-type (cadr ops)) 'insert-before))))) + +(ert-deftest llm-tools--hl-apply-replace-single-line () + (let* ((content "line1\nline2\nline3\n") + (file (make-temp-file nil nil nil content)) + (patch (format "@@ test\n= 2%s..2%s\n~REPLACED" (elt 1 (llm-tools--hl-hash (string-lines content)))))) + (unwind-protect + (progn + (llm-tools--hl-apply patch) + (should (string= (llm-tools--hl-file-read file) + (string-join + (list (format "1%s|line1" (llm-tools--hl-hash "line1")) + (format "2%s|REPLACED" (llm-tools--hl-hash "REPLACED")) + (format "3%s|line3" (llm-tools--hl-hash "line3"))) + "\n")))) + (delete-file file)))) + +(ert-deftest llm-tools--hl-apply-insert-after () + (let* ((content "A\nB\nC\n") + (file (make-temp-file nil nil nil content)) + (anchor (format "+ %s" (llm-tools--hl-file-line-hash file 1))) + (patch (concat "@@ test\n" anchor "\n~NEW"))) + (unwind-protect + (progn + (llm-tools--hl-apply patch) + (let ((lines (llm-tools--hl-file-lines file))) + (should (equal lines '("A" "NEW" "B" "C"))))) + (delete-file file)))) + +(ert-deftest llm-tools--hl-apply-insert-before () + (let* ((content "A\nB\nC\n") + (file (make-temp-file nil nil nil content)) + (anchor (format "< %s" (llm-tools--hl-file-line-hash file 3))) + (patch (concat "@@ test\n" anchor "\n~NEW"))) + (unwind-protect + (progn + (llm-tools--hl-apply patch) + (let ((lines (llm-tools--hl-file-lines file))) + (should (equal lines '("A" "B" "NEW" "C"))))) + (delete-file file)))) + +(ert-deftest llm-tools--hl-apply-append-to-eof () + (let* ((content "A\nB\n") + (file (make-temp-file nil nil nil content)) + (patch "@@ test\n+ EOF\n~APPENDED")) + (unwind-protect + (progn + (llm-tools--hl-apply patch) + (let ((lines (llm-tools--hl-file-lines file))) + (should (equal lines '("A" "B" "APPENDED"))))) + (delete-file file)))) + +(ert-deftest llm-tools--hl-apply-delete () + (let* ((content "A\nB\nC\n") + (file (make-temp-file nil nil nil content)) + (anchor (llm-tools--hl-file-line-hash file 2)) + (patch (format "@@ test\n- 2%s..2%s" anchor anchor))) + (unwind-protect + (progn + (llm-tools--hl-apply patch) + (let ((lines (llm-tools--hl-file-lines file))) + (should (equal lines '("A" "C"))))) + (delete-file file)))) + +(ert-deftest llm-tools--hl-apply-blank-line () + "Replace with no payload produces a blank line." + (let* ((content "A\nB\nC\n") + (file (make-temp-file nil nil nil content)) + (anchor (llm-tools--hl-file-line-hash file 2)) + (patch (format "@@ test\n= 2%s..2%s" anchor anchor))) + (unwind-protect + (progn + (llm-tools--hl-apply patch) + (let ((lines (llm-tools--hl-file-lines file))) + (should (equal lines '("A" "" "C"))))) + (delete-file file)))) + +(ert-deftest llm-tools--hl-apply-offset-tracking () + "Multiple ops in one file: verify offset adjusts subsequent line numbers." + (let* ((content "A\nB\nC\nD\nE\n") + (file (make-temp-file nil nil nil content)) + (a1 (llm-tools--hl-file-line-hash file 1)) + (a3 (llm-tools--hl-file-line-hash file 3)) + ;; Insert 2 lines after line 1, then delete original line 3 + (patch (format "@@ test\n+ 1%s\n~X\n~Y\n- 3%s..3%s" a1 a3 a3))) + (unwind-protect + (progn + (llm-tools--hl-apply patch) + ;; After insert: A X Y B C D E (offset = +2) + ;; Original line 3 (C) is now at index 5 (1-based). Delete it. + ;; Result: A X Y B D E + (let ((lines (llm-tools--hl-file-lines file))) + (should (equal lines '("A" "X" "Y" "B" "D" "E"))))) + (delete-file file)))) + +(ert-deftest llm-tools--hl-apply-multi-file () + "Two files in one patch both get modified." + (let* ((file1 (make-temp-file nil nil nil "A\nB\n")) + (file2 (make-temp-file nil nil nil "X\nY\n")) + (a1 (llm-tools--hl-file-line-hash file1 1)) + (a2 (llm-tools--hl-file-line-hash file2 2)) + (patch (format "@@ %s\n+ 1%s\n~NEW1\n@@ %s\n= 2%s..2%s\n~NEW2" + file1 a1 file2 a2 a2))) + (unwind-protect + (progn + (llm-tools--hl-apply patch) + (should (equal (llm-tools--hl-file-lines file1) '("A" "NEW1" "B"))) + (should (equal (llm-tools--hl-file-lines file2) '("X" "NEW2")))) + (delete-file file1) + (delete-file file2)))) + +(ert-deftest llm-tools--hl-apply-multi-line-payload () + "Replace with many payload lines." + (let* ((content "A\nB\n") + (file (make-temp-file nil nil nil content)) + (a1 (llm-tools--hl-file-line-hash file 1)) + (patch (format "@@ test\n= 1%s..1%s\n~ONE\n~TWO\n~THREE" a1 a1))) + (unwind-protect + (progn + (llm-tools--hl-apply patch) + (let ((lines (llm-tools--hl-file-lines file))) + (should (equal lines '("ONE" "TWO" "THREE" "B"))))) + (delete-file file)))) diff --git a/llm-tools-hl.el b/llm-tools-hl.el index b4ca40a..808b837 100644 --- a/llm-tools-hl.el +++ b/llm-tools-hl.el @@ -97,7 +97,7 @@ Each element is a line number and two-character hash, like `1vx'." (defun llm-tools--hl-file-line-hash (file n) "Return the hash of the Nth line in FILE." (let ((lines (llm-tools--hl-file-lines file))) - (llm-tools--hl-hash (elt lines (1- n))))) + (llm-tools--hl-hash (elt lines (1- n))))) ;;;; Diff Parsing (defun llm-tools--split-patch-sections (patch) @@ -126,7 +126,7 @@ ANCHORS is a sorted, deduplicated list of all line anchors referenced by the operations in that file's sections. Each anchor is a string like \"5ff\" or a list of two strings from a range like (\"3gv\" \"6be\")." - (let ((sections (split-patch-sections patch))) + (let ((sections (llm-tools--split-patch-sections patch))) (delete-dups (mapcar (lambda (patch) (cons (car patch) @@ -151,12 +151,17 @@ line in the on-disk file." (let ((patch-anchors (llm-tools--hl-get-anchors patch))) (mapcar (lambda (section) (let* ((file (car section)) - (anchors (delete "EOF" (cdr section))) + (anchors (delete "BOF" (delete "EOF" (cdr section)))) (hl (llm-tools--hl-file-read-hashes file))) ;; cl-subsetp uses eql by default which compares by identity and not content (cons file (cl-subsetp anchors hl :test #'equal)))) patch-anchors))) +;; TODO this should also validate the operators (e.g. any invalid +;; op-chars, payload given before all other operations, no payload +;; after +/< or payload given to -, or anchors without line +;; number). it should return a string saying it's a malformed patch +;; and what went wrong. (defun llm-tools--hl-verify (patch) "Verify if the files in PATCH need to be re-read. Returns a string message that states which files need to be re-read. @@ -171,21 +176,158 @@ Empty string means no files need to be re-read." (llm-tools--hl-verify-anchors patch)))) "\n")) +(cl-defstruct hl-op + (type nil) ; 'insert-after 'insert-before 'delete 'replace + (anchor nil) ; string like "4ei" or "EOF" for +/-/< + (range nil) ; (start . end) strings for - and = + (payload nil)) ; list of strings (nil is no payload) + +(defun llm-tools--hl-anchor-to-line-num (anchor) + "Extract the line number from ANCHOR like \"5ff\"." + (string-to-number (substring anchor 0 (- (length anchor) 2)))) + +(defun llm-tools--hl-parse-op-line (line) + "Parse a single operation LINE into a `hl-op` struct. +LINE should start with `+`, `<`, `-`, or `=`." + (pcase (substring line 0 1) + ("+" + (make-hl-op :type 'insert-after + :anchor (substring line 2))) + ("<" + (make-hl-op :type 'insert-before + :anchor (substring line 2))) + ("-" + (make-hl-op :type 'delete + :range (split-string (substring line 2) "\\.\\."))) + ("=" + (make-hl-op :type 'replace + :range (split-string (substring line 2) "\\.\\."))))) + (defun llm-tools--hl-parse-section (section) - "Parses operation and optional payload from SECTION." - ) + "Parse SECTION (body after `@@ PATH`) into a list of `hl-op` structs. +Skips comment lines (starting with `#`) and blank lines." + (let (ops op) + (dolist (line (string-lines section)) + (when (string-match "\\S-" line) + (let ((op-char (substring line 0 1))) + (pcase op-char + ("~" + (when op + (setf (hl-op-payload op) + (append (hl-op-payload op) + (list (substring line 1)))))) + ("#" nil) + (_ + (when op (push op ops)) + (setq op (llm-tools--hl-parse-op-line line)))))) + (when op (push op ops)) + (nreverse ops)))) + +(defun llm-tools--hl-group-by-file (sections) + "Group SECTIONS by file path. +Returns an alist of (FILENAME . SECTION-LIST) where SECTION-LIST +contains all (PATH . REST) cons cells from SECTIONS that target +FILENAME." + (let (grouped) + (dolist (sec sections) + (let ((existing (assoc (car sec) grouped))) + (if existing + (push sec (cdr existing)) + (push (cons (car sec) (list sec)) grouped)))) + grouped)) + +(defun llm-tools--hl-apply-one-op (op content offset) + "Apply a single OP to CONTENT (list of strings) at the given OFFSET. +Returns a cons cell (NEW-CONTENT . NEW-OFFSET)." + (pcase (hl-op-type op) + ('insert-after + (let* ((anchor (hl-op-anchor op)) + (payload (hl-op-payload op)) + (line-num (if (string= anchor "EOF") + (length content) + (+ (llm-tools--hl-anchor-to-line-num anchor) + offset))) + (idx (1- line-num))) + (cons (append (cl-subseq content 0 (1+ idx)) + payload + (cl-subseq content (1+ idx))) + (+ offset (length payload))))) + ('insert-before + (let* ((anchor (hl-op-anchor op)) + (payload (hl-op-payload op)) + (line-num (if (string= anchor "BOF") + 1 + (+ (llm-tools--hl-anchor-to-line-num anchor) + offset))) + (idx (1- line-num))) + (cons (append (cl-subseq content 0 idx) + payload + (cl-subseq content idx)) + (+ offset (length payload))))) + ('delete + (let* ((range (hl-op-range op)) + (a (+ (llm-tools--hl-anchor-to-line-num (car range)) offset)) + (b (+ (llm-tools--hl-anchor-to-line-num (cadr range)) offset))) + (cons (append (cl-subseq content 0 (1- a)) + (cl-subseq content b)) + (- offset (- b a 1))))) + ('replace + (let* ((range (hl-op-range op)) + (a (+ (llm-tools--hl-anchor-to-line-num (car range)) offset)) + (b (+ (llm-tools--hl-anchor-to-line-num (cadr range)) offset)) + (payload (or (hl-op-payload op) + '("")))) + (cons (append (cl-subseq content 0 (1- a)) + payload + (cl-subseq content b)) + (+ offset (- (length payload) (- b a 1)))))))) + +(defun llm-tools--hl-apply-ops (ops content) + "Apply a list of OPS to CONTENT (list of strings). +Returns the modified list of strings." + (let ((offset 0)) + (dolist (op ops content) + (let ((result (llm-tools--hl-apply-one-op op content offset))) + (setq content (car result)) + (setq offset (cdr result)))))) (defun llm-tools--hl-apply (patch) - "TODO rewrite docstring. Applies a patch." - ;; when performing each operation, I need to make sure that the line - ;; numbers of subsequent operations in same file also get updated. - (let ((sections (split-patch-sections patch))) - (llm-tools--hl-parse sections))) + "Apply all operations in PATCH to their respective files. +Operations within a file are applied sequentially, with line numbers +adjusted for prior insertions/deletions via a running offset." + (dolist (file-group (llm-tools--hl-group-by-file + (llm-tools--split-patch-sections patch))) + (let* ((file (car file-group)) + (ops (mapcan (lambda (sec) + (llm-tools--hl-parse-section (cdr sec))) + (cdr file-group))) + (content (llm-tools--hl-apply-ops + ops + (llm-tools--hl-file-lines file)))) + (with-temp-buffer + (insert (string-join content "\n")) + (write-region (point-min) (point-max) file))))) (defun llm-tools--hl-edit (patch) - "TODO write better docstring. First verifies if any files in PATCH needs -to be re-read. Then, applies the changes in PATCH. If any files need to -be re-read, the edit operation is cancelled for all files." + "Apply a hashline PATCH to the files it references. + +PATCH is a string containing one or more sections, each starting with +`@@ FILEPATH' followed by operations: + + ANCHOR Insert lines after ANCHOR (or `EOF' to append) + < ANCHOR Insert lines before ANCHOR (or `BOF' to prepend) + - A..B Delete lines from A to B + = A..B Replace lines from A to B with the following payload + ~TEXT Payload line for the preceding operation + +ANCHOR is a hashline string like `5ff' (line number + 2-char hash) or +`BOF' / `EOF'. Lines are written directly to disk. + +Before applying, verifies that every anchor in PATCH still matches the +current file contents. If any file has drifted, the entire operation is +aborted and an error message is returned identifying the affected files. + +Returns a success message if all operations applied, or an error string +if verification failed." (let ((msg (llm-tools--hl-verify patch))) (if (string= msg "") (progn -- cgit v1.2.3