https://github.com/goldsteinn updated https://github.com/llvm/llvm-project/pull/112792
>From 802764e879862541e205ba1a070824b71d2fef9a Mon Sep 17 00:00:00 2001 From: Noah Goldstein <goldstein....@gmail.com> Date: Thu, 17 Oct 2024 17:31:24 -0500 Subject: [PATCH 01/12] [emacs][clang-format] Add elisp API for clang-format on git diffs New proposed function `clang-format-git-diffs`. It is the same as calling `clang-format-region` on all diffs between the content of a buffer-file and the content of the file at git revision HEAD. This is essentially the same thing as: `git-clang-format -f {filename}` If the current buffer is saved. The motivation is many project (LLVM included) both have code that is non-compliant with there clang-format style and disallow unrelated format diffs in PRs. This means users can't just run `clang-format-buffer` on the buffer they are working on, and need to manually go through all the regions by hand to get them formatted. This is both an error prone and annoying workflow. --- clang/tools/clang-format/clang-format.el | 159 ++++++++++++++++++++--- 1 file changed, 144 insertions(+), 15 deletions(-) diff --git a/clang/tools/clang-format/clang-format.el b/clang/tools/clang-format/clang-format.el index fb943b7b722f8a..d3f874de41c550 100644 --- a/clang/tools/clang-format/clang-format.el +++ b/clang/tools/clang-format/clang-format.el @@ -146,18 +146,97 @@ is a zero-based file offset, assuming ‘utf-8-unix’ coding." (lambda (byte &optional _quality _coding-system) (byte-to-position (1+ byte))))) -;;;###autoload -(defun clang-format-region (start end &optional style assume-file-name) - "Use clang-format to format the code between START and END according to STYLE. -If called interactively uses the region or the current statement if there is no -no active region. If no STYLE is given uses `clang-format-style'. Use -ASSUME-FILE-NAME to locate a style config file, if no ASSUME-FILE-NAME is given -uses the function `buffer-file-name'." - (interactive - (if (use-region-p) - (list (region-beginning) (region-end)) - (list (point) (point)))) - +(defun clang-format--git-diffs-get-diff-lines (file-orig file-new) + "Return all line regions that contain diffs between FILE-ORIG and +FILE-NEW. If there is no diff 'nil' is returned. Otherwise the +return is a 'list' of lines in the format '--lines=<start>:<end>' +which can be passed directly to 'clang-format'" + ;; Temporary buffer for output of diff. + (with-temp-buffer + (let ((status (call-process + "diff" + nil + (current-buffer) + nil + ;; Binary diff has different behaviors that we + ;; aren't interested in. + "-a" + ;; Printout changes as only the line groups. + "--changed-group-format=--lines=%dF:%dL " + ;; Ignore unchanged content. + "--unchanged-group-format=" + file-orig + file-new + ) + ) + (stderr (concat (if (zerop (buffer-size)) "" ": ") + (buffer-substring-no-properties + (point-min) (line-end-position))))) + (when (stringp status) + (error "(diff killed by signal %s%s)" status stderr)) + (unless (= status 0) + (unless (= status 1) + (error "(diff returned unsuccessfully %s%s)" status stderr))) + + + (if (= status 0) + ;; Status == 0 -> no Diff. + nil + (progn + ;; Split "--lines=<S0>:<E0>... --lines=<SN>:<SN>" output to + ;; a list for return. + (s-split + " " + (string-trim + (buffer-substring-no-properties + (point-min) (point-max))))))))) + +(defun clang-format--git-diffs-get-git-head-file () + "Returns a temporary file with the content of 'buffer-file-name' at +git revision HEAD. If the current buffer is either not a file or not +in a git repo, this results in an error" + ;; Needs current buffer to be a file + (unless (buffer-file-name) + (error "Buffer is not visiting a file")) + ;; Need to be able to find version control (git) root + (unless (vc-root-dir) + (error "File not known to git")) + ;; Need version control to in fact be git + (unless (string-equal (vc-backend (buffer-file-name)) "Git") + (error "Not using git")) + + (let ((tmpfile-git-head (make-temp-file "clang-format-tmp-git-head-content"))) + ;; Get filename relative to git root + (let ((git-file-name (substring + (expand-file-name (buffer-file-name)) + (string-width (expand-file-name (vc-root-dir))) + nil))) + (let ((status (call-process + "git" + nil + `(:file, tmpfile-git-head) + nil + "show" (concat "HEAD:" git-file-name))) + (stderr (with-temp-buffer + (unless (zerop (cadr (insert-file-contents tmpfile-git-head))) + (insert ": ")) + (buffer-substring-no-properties + (point-min) (line-end-position))))) + (when (stringp status) + (error "(git show HEAD:%s killed by signal %s%s)" + git-file-name status stderr)) + (unless (zerop status) + (error "(git show HEAD:%s returned unsuccessfully %s%s)" + git-file-name status stderr)))) + ;; Return temporary file so we can diff it. + tmpfile-git-head)) + +(defun clang-format--region-impl (start end &optional style assume-file-name lines) + "Common implementation for 'clang-format-buffer', +'clang-format-region', and 'clang-format-git-diffs'. START and END +refer to the region to be formatter. STYLE and ASSUME-FILE-NAME are +used for configuring the clang-format. And LINES is used to pass +specific locations for reformatting (i.e diff locations)." (unless style (setq style clang-format-style)) @@ -190,8 +269,12 @@ uses the function `buffer-file-name'." (list "--assume-filename" assume-file-name)) ,@(and style (list "--style" style)) "--fallback-style" ,clang-format-fallback-style - "--offset" ,(number-to-string file-start) - "--length" ,(number-to-string (- file-end file-start)) + ,@(and lines lines) + ,@(and (not lines) + (list + "--offset" (number-to-string file-start) + "--length" (number-to-string + (- file-end file-start)))) "--cursor" ,(number-to-string cursor)))) (stderr (with-temp-buffer (unless (zerop (cadr (insert-file-contents temp-file))) @@ -219,6 +302,48 @@ uses the function `buffer-file-name'." (delete-file temp-file) (when (buffer-name temp-buffer) (kill-buffer temp-buffer))))) +;;;###autoload +(defun clang-format-git-diffs (&optional style assume-file-name) + "The same as 'clang-format-buffer' but only operates on the git +diffs from HEAD in the buffer. If no STYLE is given uses +`clang-format-style'. Use ASSUME-FILE-NAME to locate a style config +file. If no ASSUME-FILE-NAME is given uses the function +`buffer-file-name'." + (interactive) + (let ((tmpfile-git-head + (clang-format--git-diffs-get-git-head-file)) + (tmpfile-curbuf (make-temp-file "clang-format-git-tmp"))) + ;; Move current buffer to a temporary file to take a diff. Even if + ;; current-buffer is backed by a file, we want to diff the buffer + ;; contents which might not be saved. + (write-region nil nil tmpfile-curbuf nil 'nomessage) + ;; Git list of lines with a diff. + (let ((diff-lines + (clang-format--git-diffs-get-diff-lines + tmpfile-git-head tmpfile-curbuf))) + ;; If we have any diffs, format them. + (when diff-lines + (clang-format--region-impl + (point-min) + (point-max) + style + assume-file-name + diff-lines))))) + +;;;###autoload +(defun clang-format-region (start end &optional style assume-file-name) + "Use clang-format to format the code between START and END according +to STYLE. If called interactively uses the region or the current +statement if there is no no active region. If no STYLE is given uses +`clang-format-style'. Use ASSUME-FILE-NAME to locate a style config +file, if no ASSUME-FILE-NAME is given uses the function +`buffer-file-name'." + (interactive + (if (use-region-p) + (list (region-beginning) (region-end)) + (list (point) (point)))) + (clang-format--region-impl start end style assume-file-name)) + ;;;###autoload (defun clang-format-buffer (&optional style assume-file-name) "Use clang-format to format the current buffer according to STYLE. @@ -226,7 +351,11 @@ If no STYLE is given uses `clang-format-style'. Use ASSUME-FILE-NAME to locate a style config file. If no ASSUME-FILE-NAME is given uses the function `buffer-file-name'." (interactive) - (clang-format-region (point-min) (point-max) style assume-file-name)) + (clang-format--region-impl + (point-min) + (point-max) + style + assume-file-name)) ;;;###autoload (defalias 'clang-format 'clang-format-region) >From 2c8395da26d2fb654ad26566fbd6db6b8523be69 Mon Sep 17 00:00:00 2001 From: Noah Goldstein <goldstein....@gmail.com> Date: Sun, 3 Nov 2024 10:31:27 -0600 Subject: [PATCH 02/12] Address V2 Comments --- clang/tools/clang-format/clang-format.el | 137 ++++++++++++++--------- 1 file changed, 83 insertions(+), 54 deletions(-) diff --git a/clang/tools/clang-format/clang-format.el b/clang/tools/clang-format/clang-format.el index d3f874de41c550..2a3d818e5db4e7 100644 --- a/clang/tools/clang-format/clang-format.el +++ b/clang/tools/clang-format/clang-format.el @@ -146,6 +146,20 @@ is a zero-based file offset, assuming ‘utf-8-unix’ coding." (lambda (byte &optional _quality _coding-system) (byte-to-position (1+ byte))))) +(defun clang-format--git-diffs-match-diff-line (line) + ;; Matching something like: + ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@" + ;; Return as "<LineStart>:<LineEnd>" + (when (string-match "^@@\s-[0-9,]+\s\\+\\([0-9]+\\)\\(,\\([0-9]+\\)\\)?\s@@$" line) + ;; If we have multi-line diff + (if (match-string 3 line) + (concat (match-string 1 line) + ":" + (number-to-string + (+ (string-to-number (match-string 1 line)) + (string-to-number (match-string 3 line))))) + (concat (match-string 1 line) ":" (match-string 1 line))))) + (defun clang-format--git-diffs-get-diff-lines (file-orig file-new) "Return all line regions that contain diffs between FILE-ORIG and FILE-NEW. If there is no diff 'nil' is returned. Otherwise the @@ -161,55 +175,60 @@ which can be passed directly to 'clang-format'" ;; Binary diff has different behaviors that we ;; aren't interested in. "-a" - ;; Printout changes as only the line groups. - "--changed-group-format=--lines=%dF:%dL " - ;; Ignore unchanged content. - "--unchanged-group-format=" + ;; Get minimal diff (copy diff config for git-clang-format) + "-U0" file-orig - file-new - ) - ) + file-new)) (stderr (concat (if (zerop (buffer-size)) "" ": ") (buffer-substring-no-properties - (point-min) (line-end-position))))) - (when (stringp status) + (point-min) (line-end-position)))) + (diff-lines '())) + (cond + ((stringp status) (error "(diff killed by signal %s%s)" status stderr)) - (unless (= status 0) - (unless (= status 1) - (error "(diff returned unsuccessfully %s%s)" status stderr))) - - - (if (= status 0) - ;; Status == 0 -> no Diff. - nil - (progn - ;; Split "--lines=<S0>:<E0>... --lines=<SN>:<SN>" output to - ;; a list for return. - (s-split - " " - (string-trim - (buffer-substring-no-properties - (point-min) (point-max))))))))) - -(defun clang-format--git-diffs-get-git-head-file () + ;; Return of 0 indicates no diff + ((= status 0) nil) + ;; Return of 1 indicates found diffs and no error + ((= status 1) + ;; Iterate through all lines in diff buffer and collect all + ;; lines in current buffer that have a diff. + (goto-char (point-min)) + (while (not (eobp)) + (let ((diff-line (clang-format--git-diffs-match-diff-line + (buffer-substring-no-properties + (line-beginning-position) + (line-end-position))))) + (when diff-line + ;; Create list line regions with diffs to pass to + ;; clang-format + (add-to-list 'diff-lines (concat "--lines=" diff-line) t))) + (forward-line 1)) + diff-lines) + ;; Any return != 0 && != 1 indicates some level of error + (t + (error "(diff returned unsuccessfully %s%s)" status stderr)))))) + +(defun clang-format--git-diffs-get-git-head-file (tmpfile-git-head) "Returns a temporary file with the content of 'buffer-file-name' at git revision HEAD. If the current buffer is either not a file or not in a git repo, this results in an error" ;; Needs current buffer to be a file (unless (buffer-file-name) (error "Buffer is not visiting a file")) - ;; Need to be able to find version control (git) root - (unless (vc-root-dir) - (error "File not known to git")) ;; Need version control to in fact be git (unless (string-equal (vc-backend (buffer-file-name)) "Git") (error "Not using git")) - (let ((tmpfile-git-head (make-temp-file "clang-format-tmp-git-head-content"))) + (let ((base-dir (vc-root-dir))) + ;; Need to be able to find version control (git) root + (unless base-dir + (error "File not known to git")) + + ;; Get filename relative to git root (let ((git-file-name (substring (expand-file-name (buffer-file-name)) - (string-width (expand-file-name (vc-root-dir))) + (string-width (expand-file-name base-dir)) nil))) (let ((status (call-process "git" @@ -227,9 +246,7 @@ in a git repo, this results in an error" git-file-name status stderr)) (unless (zerop status) (error "(git show HEAD:%s returned unsuccessfully %s%s)" - git-file-name status stderr)))) - ;; Return temporary file so we can diff it. - tmpfile-git-head)) + git-file-name status stderr)))))) (defun clang-format--region-impl (start end &optional style assume-file-name lines) "Common implementation for 'clang-format-buffer', @@ -302,6 +319,7 @@ specific locations for reformatting (i.e diff locations)." (delete-file temp-file) (when (buffer-name temp-buffer) (kill-buffer temp-buffer))))) + ;;;###autoload (defun clang-format-git-diffs (&optional style assume-file-name) "The same as 'clang-format-buffer' but only operates on the git @@ -310,25 +328,36 @@ diffs from HEAD in the buffer. If no STYLE is given uses file. If no ASSUME-FILE-NAME is given uses the function `buffer-file-name'." (interactive) - (let ((tmpfile-git-head - (clang-format--git-diffs-get-git-head-file)) - (tmpfile-curbuf (make-temp-file "clang-format-git-tmp"))) - ;; Move current buffer to a temporary file to take a diff. Even if - ;; current-buffer is backed by a file, we want to diff the buffer - ;; contents which might not be saved. - (write-region nil nil tmpfile-curbuf nil 'nomessage) - ;; Git list of lines with a diff. - (let ((diff-lines - (clang-format--git-diffs-get-diff-lines - tmpfile-git-head tmpfile-curbuf))) - ;; If we have any diffs, format them. - (when diff-lines - (clang-format--region-impl - (point-min) - (point-max) - style - assume-file-name - diff-lines))))) + (let ((tmpfile-git-head nil) + (tmpfile-curbuf nil)) + (unwind-protect + (progn + (setq tmpfile-git-head + (make-temp-file "clang-format-git-tmp-head-content")) + (clang-format--git-diffs-get-git-head-file tmpfile-git-head) + ;; Move current buffer to a temporary file to take a + ;; diff. Even if current-buffer is backed by a file, we + ;; want to diff the buffer contents which might not be + ;; saved. + (setq tmpfile-curbuf (make-temp-file "clang-format-git-tmp")) + (write-region nil nil tmpfile-curbuf nil 'nomessage) + ;; Git list of lines with a diff. + (let ((diff-lines + (clang-format--git-diffs-get-diff-lines + tmpfile-git-head tmpfile-curbuf))) + ;; If we have any diffs, format them. + (when diff-lines + (clang-format--region-impl + (point-min) + (point-max) + style + assume-file-name + diff-lines)))) + (progn + ;; Cleanup temporary files + (when tmpfile-git-head (delete-file tmpfile-git-head)) + (when tmpfile-curbuf (delete-file tmpfile-curbuf)))))) + ;;;###autoload (defun clang-format-region (start end &optional style assume-file-name) >From fc05d68895d7af670b03a8b0aa6790a25730d6f1 Mon Sep 17 00:00:00 2001 From: Noah Goldstein <goldstein....@gmail.com> Date: Sun, 3 Nov 2024 10:36:16 -0600 Subject: [PATCH 03/12] Rename 'git' -> 'vc' and 'diffs' -> 'diff' --- clang/tools/clang-format/clang-format.el | 53 ++++++++++++------------ 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/clang/tools/clang-format/clang-format.el b/clang/tools/clang-format/clang-format.el index 2a3d818e5db4e7..c1a3984df1b4f0 100644 --- a/clang/tools/clang-format/clang-format.el +++ b/clang/tools/clang-format/clang-format.el @@ -146,7 +146,7 @@ is a zero-based file offset, assuming ‘utf-8-unix’ coding." (lambda (byte &optional _quality _coding-system) (byte-to-position (1+ byte))))) -(defun clang-format--git-diffs-match-diff-line (line) +(defun clang-format--vc-diff-match-diff-line (line) ;; Matching something like: ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@" ;; Return as "<LineStart>:<LineEnd>" @@ -160,7 +160,7 @@ is a zero-based file offset, assuming ‘utf-8-unix’ coding." (string-to-number (match-string 3 line))))) (concat (match-string 1 line) ":" (match-string 1 line))))) -(defun clang-format--git-diffs-get-diff-lines (file-orig file-new) +(defun clang-format--vc-diff-get-diff-lines (file-orig file-new) "Return all line regions that contain diffs between FILE-ORIG and FILE-NEW. If there is no diff 'nil' is returned. Otherwise the return is a 'list' of lines in the format '--lines=<start>:<end>' @@ -194,7 +194,7 @@ which can be passed directly to 'clang-format'" ;; lines in current buffer that have a diff. (goto-char (point-min)) (while (not (eobp)) - (let ((diff-line (clang-format--git-diffs-match-diff-line + (let ((diff-line (clang-format--vc-diff-match-diff-line (buffer-substring-no-properties (line-beginning-position) (line-end-position))))) @@ -208,14 +208,15 @@ which can be passed directly to 'clang-format'" (t (error "(diff returned unsuccessfully %s%s)" status stderr)))))) -(defun clang-format--git-diffs-get-git-head-file (tmpfile-git-head) - "Returns a temporary file with the content of 'buffer-file-name' at -git revision HEAD. If the current buffer is either not a file or not -in a git repo, this results in an error" +(defun clang-format--vc-diff-get-vc-head-file (tmpfile-vc-head) + "Stores the contents of 'buffer-file-name' at vc revision HEAD into +'tmpfile-vc-head'. If the current buffer is either not a file or not +in a vc repo, this results in an error. Currently git is the only +supported vc." ;; Needs current buffer to be a file (unless (buffer-file-name) (error "Buffer is not visiting a file")) - ;; Need version control to in fact be git + ;; Only version control currently supported is Git (unless (string-equal (vc-backend (buffer-file-name)) "Git") (error "Not using git")) @@ -226,31 +227,31 @@ in a git repo, this results in an error" ;; Get filename relative to git root - (let ((git-file-name (substring + (let ((vc-file-name (substring (expand-file-name (buffer-file-name)) (string-width (expand-file-name base-dir)) nil))) (let ((status (call-process "git" nil - `(:file, tmpfile-git-head) + `(:file, tmpfile-vc-head) nil - "show" (concat "HEAD:" git-file-name))) + "show" (concat "HEAD:" vc-file-name))) (stderr (with-temp-buffer - (unless (zerop (cadr (insert-file-contents tmpfile-git-head))) + (unless (zerop (cadr (insert-file-contents tmpfile-vc-head))) (insert ": ")) (buffer-substring-no-properties (point-min) (line-end-position))))) (when (stringp status) (error "(git show HEAD:%s killed by signal %s%s)" - git-file-name status stderr)) + vc-file-name status stderr)) (unless (zerop status) (error "(git show HEAD:%s returned unsuccessfully %s%s)" - git-file-name status stderr)))))) + vc-file-name status stderr)))))) (defun clang-format--region-impl (start end &optional style assume-file-name lines) "Common implementation for 'clang-format-buffer', -'clang-format-region', and 'clang-format-git-diffs'. START and END +'clang-format-region', and 'clang-format-vc-diff'. START and END refer to the region to be formatter. STYLE and ASSUME-FILE-NAME are used for configuring the clang-format. And LINES is used to pass specific locations for reformatting (i.e diff locations)." @@ -321,30 +322,30 @@ specific locations for reformatting (i.e diff locations)." ;;;###autoload -(defun clang-format-git-diffs (&optional style assume-file-name) - "The same as 'clang-format-buffer' but only operates on the git +(defun clang-format-vc-diff (&optional style assume-file-name) + "The same as 'clang-format-buffer' but only operates on the vc diffs from HEAD in the buffer. If no STYLE is given uses `clang-format-style'. Use ASSUME-FILE-NAME to locate a style config file. If no ASSUME-FILE-NAME is given uses the function `buffer-file-name'." (interactive) - (let ((tmpfile-git-head nil) + (let ((tmpfile-vc-head nil) (tmpfile-curbuf nil)) (unwind-protect (progn - (setq tmpfile-git-head - (make-temp-file "clang-format-git-tmp-head-content")) - (clang-format--git-diffs-get-git-head-file tmpfile-git-head) + (setq tmpfile-vc-head + (make-temp-file "clang-format-vc-tmp-head-content")) + (clang-format--vc-diff-get-vc-head-file tmpfile-vc-head) ;; Move current buffer to a temporary file to take a ;; diff. Even if current-buffer is backed by a file, we ;; want to diff the buffer contents which might not be ;; saved. - (setq tmpfile-curbuf (make-temp-file "clang-format-git-tmp")) + (setq tmpfile-curbuf (make-temp-file "clang-format-vc-tmp")) (write-region nil nil tmpfile-curbuf nil 'nomessage) - ;; Git list of lines with a diff. + ;; Get list of lines with a diff. (let ((diff-lines - (clang-format--git-diffs-get-diff-lines - tmpfile-git-head tmpfile-curbuf))) + (clang-format--vc-diff-get-diff-lines + tmpfile-vc-head tmpfile-curbuf))) ;; If we have any diffs, format them. (when diff-lines (clang-format--region-impl @@ -355,7 +356,7 @@ file. If no ASSUME-FILE-NAME is given uses the function diff-lines)))) (progn ;; Cleanup temporary files - (when tmpfile-git-head (delete-file tmpfile-git-head)) + (when tmpfile-vc-head (delete-file tmpfile-vc-head)) (when tmpfile-curbuf (delete-file tmpfile-curbuf)))))) >From 36aa80bd7f1b35946252a6b449014529c3e23101 Mon Sep 17 00:00:00 2001 From: Noah Goldstein <goldstein....@gmail.com> Date: Sun, 3 Nov 2024 23:13:11 -0600 Subject: [PATCH 04/12] Complete sentences/byte-compile/refactor vc check --- clang/tools/clang-format/clang-format.el | 105 ++++++++++++----------- 1 file changed, 54 insertions(+), 51 deletions(-) diff --git a/clang/tools/clang-format/clang-format.el b/clang/tools/clang-format/clang-format.el index c1a3984df1b4f0..f9ebede1b215c6 100644 --- a/clang/tools/clang-format/clang-format.el +++ b/clang/tools/clang-format/clang-format.el @@ -147,7 +147,7 @@ is a zero-based file offset, assuming ‘utf-8-unix’ coding." (byte-to-position (1+ byte))))) (defun clang-format--vc-diff-match-diff-line (line) - ;; Matching something like: + ;; We are matching something like: ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@" ;; Return as "<LineStart>:<LineEnd>" (when (string-match "^@@\s-[0-9,]+\s\\+\\([0-9]+\\)\\(,\\([0-9]+\\)\\)?\s@@$" line) @@ -162,10 +162,10 @@ is a zero-based file offset, assuming ‘utf-8-unix’ coding." (defun clang-format--vc-diff-get-diff-lines (file-orig file-new) "Return all line regions that contain diffs between FILE-ORIG and -FILE-NEW. If there is no diff 'nil' is returned. Otherwise the -return is a 'list' of lines in the format '--lines=<start>:<end>' -which can be passed directly to 'clang-format'" - ;; Temporary buffer for output of diff. +FILE-NEW. If there is no diff ‘nil’ is returned. Otherwise the +return is a ‘list’ of lines in the format ‘--lines=<start>:<end>’ +which can be passed directly to ‘clang-format’." + ;; Use temporary buffer for output of diff. (with-temp-buffer (let ((status (call-process "diff" @@ -175,7 +175,7 @@ which can be passed directly to 'clang-format'" ;; Binary diff has different behaviors that we ;; aren't interested in. "-a" - ;; Get minimal diff (copy diff config for git-clang-format) + ;; Get minimal diff (copy diff config for git-clang-format). "-U0" file-orig file-new)) @@ -186,9 +186,9 @@ which can be passed directly to 'clang-format'" (cond ((stringp status) (error "(diff killed by signal %s%s)" status stderr)) - ;; Return of 0 indicates no diff + ;; Return of 0 indicates no diff. ((= status 0) nil) - ;; Return of 1 indicates found diffs and no error + ;; Return of 1 indicates found diffs and no error. ((= status 1) ;; Iterate through all lines in diff buffer and collect all ;; lines in current buffer that have a diff. @@ -200,58 +200,61 @@ which can be passed directly to 'clang-format'" (line-end-position))))) (when diff-line ;; Create list line regions with diffs to pass to - ;; clang-format - (add-to-list 'diff-lines (concat "--lines=" diff-line) t))) + ;; clang-format. + (push (concat "--lines=" diff-line) diff-lines))) (forward-line 1)) - diff-lines) - ;; Any return != 0 && != 1 indicates some level of error + (reverse diff-lines)) + ;; Any return != 0 && != 1 indicates some level of error. (t (error "(diff returned unsuccessfully %s%s)" status stderr)))))) (defun clang-format--vc-diff-get-vc-head-file (tmpfile-vc-head) - "Stores the contents of 'buffer-file-name' at vc revision HEAD into -'tmpfile-vc-head'. If the current buffer is either not a file or not + "Stores the contents of ‘buffer-file-name’ at vc revision HEAD into +‘tmpfile-vc-head’. If the current buffer is either not a file or not in a vc repo, this results in an error. Currently git is the only supported vc." - ;; Needs current buffer to be a file + ;; We need the current buffer to be a file. (unless (buffer-file-name) (error "Buffer is not visiting a file")) - ;; Only version control currently supported is Git - (unless (string-equal (vc-backend (buffer-file-name)) "Git") - (error "Not using git")) - (let ((base-dir (vc-root-dir))) - ;; Need to be able to find version control (git) root + (let ((base-dir (vc-root-dir)) + (backend (vc-backend (buffer-file-name)))) + ;; We need to be able to find version control (git) root. (unless base-dir (error "File not known to git")) + (cond + ((string-equal backend "Git") + ;; Get the filename relative to git root. + (let ((vc-file-name (substring + (expand-file-name (buffer-file-name)) + (string-width (expand-file-name base-dir)) + nil))) + (let ((status (call-process + "git" + nil + `(:file, tmpfile-vc-head) + nil + "show" (concat "HEAD:" vc-file-name))) + (stderr (with-temp-buffer + (unless (zerop (cadr (insert-file-contents tmpfile-vc-head))) + (insert ": ")) + (buffer-substring-no-properties + (point-min) (line-end-position))))) + (when (stringp status) + (error "(git show HEAD:%s killed by signal %s%s)" + vc-file-name status stderr)) + (unless (zerop status) + (error "(git show HEAD:%s returned unsuccessfully %s%s)" + vc-file-name status stderr))))) + (t + (error + "Version control %s isn't supported, currently supported backends: git" + backend))))) - ;; Get filename relative to git root - (let ((vc-file-name (substring - (expand-file-name (buffer-file-name)) - (string-width (expand-file-name base-dir)) - nil))) - (let ((status (call-process - "git" - nil - `(:file, tmpfile-vc-head) - nil - "show" (concat "HEAD:" vc-file-name))) - (stderr (with-temp-buffer - (unless (zerop (cadr (insert-file-contents tmpfile-vc-head))) - (insert ": ")) - (buffer-substring-no-properties - (point-min) (line-end-position))))) - (when (stringp status) - (error "(git show HEAD:%s killed by signal %s%s)" - vc-file-name status stderr)) - (unless (zerop status) - (error "(git show HEAD:%s returned unsuccessfully %s%s)" - vc-file-name status stderr)))))) - (defun clang-format--region-impl (start end &optional style assume-file-name lines) - "Common implementation for 'clang-format-buffer', -'clang-format-region', and 'clang-format-vc-diff'. START and END + "Common implementation for ‘clang-format-buffer’, +‘clang-format-region’, and ‘clang-format-vc-diff’. START and END refer to the region to be formatter. STYLE and ASSUME-FILE-NAME are used for configuring the clang-format. And LINES is used to pass specific locations for reformatting (i.e diff locations)." @@ -323,11 +326,11 @@ specific locations for reformatting (i.e diff locations)." ;;;###autoload (defun clang-format-vc-diff (&optional style assume-file-name) - "The same as 'clang-format-buffer' but only operates on the vc + "The same as ‘clang-format-buffer’ but only operates on the vc diffs from HEAD in the buffer. If no STYLE is given uses -`clang-format-style'. Use ASSUME-FILE-NAME to locate a style config +‘clang-format-style’. Use ASSUME-FILE-NAME to locate a style config file. If no ASSUME-FILE-NAME is given uses the function -`buffer-file-name'." +‘buffer-file-name’." (interactive) (let ((tmpfile-vc-head nil) (tmpfile-curbuf nil)) @@ -336,13 +339,13 @@ file. If no ASSUME-FILE-NAME is given uses the function (setq tmpfile-vc-head (make-temp-file "clang-format-vc-tmp-head-content")) (clang-format--vc-diff-get-vc-head-file tmpfile-vc-head) - ;; Move current buffer to a temporary file to take a + ;; Move the current buffer to a temporary file to take a ;; diff. Even if current-buffer is backed by a file, we ;; want to diff the buffer contents which might not be ;; saved. (setq tmpfile-curbuf (make-temp-file "clang-format-vc-tmp")) (write-region nil nil tmpfile-curbuf nil 'nomessage) - ;; Get list of lines with a diff. + ;; Get a list of lines with a diff. (let ((diff-lines (clang-format--vc-diff-get-diff-lines tmpfile-vc-head tmpfile-curbuf))) @@ -355,7 +358,7 @@ file. If no ASSUME-FILE-NAME is given uses the function assume-file-name diff-lines)))) (progn - ;; Cleanup temporary files + ;; Cleanup temporary files we created. (when tmpfile-vc-head (delete-file tmpfile-vc-head)) (when tmpfile-curbuf (delete-file tmpfile-curbuf)))))) >From fcf504f46ee3a46b9f6e8a21a25f07db2d4b215f Mon Sep 17 00:00:00 2001 From: Noah Goldstein <goldstein....@gmail.com> Date: Mon, 4 Nov 2024 10:06:54 -0600 Subject: [PATCH 05/12] use diff-no-select --- clang/tools/clang-format/clang-format.el | 168 +++++++++-------------- 1 file changed, 64 insertions(+), 104 deletions(-) diff --git a/clang/tools/clang-format/clang-format.el b/clang/tools/clang-format/clang-format.el index f9ebede1b215c6..0d732683d94f0a 100644 --- a/clang/tools/clang-format/clang-format.el +++ b/clang/tools/clang-format/clang-format.el @@ -160,55 +160,31 @@ is a zero-based file offset, assuming ‘utf-8-unix’ coding." (string-to-number (match-string 3 line))))) (concat (match-string 1 line) ":" (match-string 1 line))))) -(defun clang-format--vc-diff-get-diff-lines (file-orig file-new) +(defun clang-format--vc-diff-compute-diff-and-get-lines (buf-orig buf-cur) "Return all line regions that contain diffs between FILE-ORIG and FILE-NEW. If there is no diff ‘nil’ is returned. Otherwise the return is a ‘list’ of lines in the format ‘--lines=<start>:<end>’ which can be passed directly to ‘clang-format’." ;; Use temporary buffer for output of diff. (with-temp-buffer - (let ((status (call-process - "diff" - nil - (current-buffer) - nil - ;; Binary diff has different behaviors that we - ;; aren't interested in. - "-a" - ;; Get minimal diff (copy diff config for git-clang-format). - "-U0" - file-orig - file-new)) - (stderr (concat (if (zerop (buffer-size)) "" ": ") + (diff-no-select buf-orig buf-cur "-a -U0" t (current-buffer)) + (let ((diff-lines '())) + ;; Iterate through all lines in diff buffer and collect all + ;; lines in current buffer that have a diff. + (goto-char (point-min)) + (while (not (eobp)) + (let ((diff-line (clang-format--vc-diff-match-diff-line (buffer-substring-no-properties - (point-min) (line-end-position)))) - (diff-lines '())) - (cond - ((stringp status) - (error "(diff killed by signal %s%s)" status stderr)) - ;; Return of 0 indicates no diff. - ((= status 0) nil) - ;; Return of 1 indicates found diffs and no error. - ((= status 1) - ;; Iterate through all lines in diff buffer and collect all - ;; lines in current buffer that have a diff. - (goto-char (point-min)) - (while (not (eobp)) - (let ((diff-line (clang-format--vc-diff-match-diff-line - (buffer-substring-no-properties - (line-beginning-position) - (line-end-position))))) - (when diff-line - ;; Create list line regions with diffs to pass to - ;; clang-format. - (push (concat "--lines=" diff-line) diff-lines))) - (forward-line 1)) - (reverse diff-lines)) - ;; Any return != 0 && != 1 indicates some level of error. - (t - (error "(diff returned unsuccessfully %s%s)" status stderr)))))) - -(defun clang-format--vc-diff-get-vc-head-file (tmpfile-vc-head) + (line-beginning-position) + (line-end-position))))) + (when diff-line + ;; Create list line regions with diffs to pass to + ;; clang-format. + (push (concat "--lines=" diff-line) diff-lines))) + (forward-line 1)) + (reverse diff-lines)))) + +(defun clang-format--vc-diff-get-diff-lines () "Stores the contents of ‘buffer-file-name’ at vc revision HEAD into ‘tmpfile-vc-head’. If the current buffer is either not a file or not in a vc repo, this results in an error. Currently git is the only @@ -217,39 +193,44 @@ supported vc." (unless (buffer-file-name) (error "Buffer is not visiting a file")) - (let ((base-dir (vc-root-dir)) + (let ((inp-buf (current-buffer)) + (base-dir (vc-root-dir)) (backend (vc-backend (buffer-file-name)))) - ;; We need to be able to find version control (git) root. - (unless base-dir - (error "File not known to git")) - (cond - ((string-equal backend "Git") - ;; Get the filename relative to git root. - (let ((vc-file-name (substring - (expand-file-name (buffer-file-name)) - (string-width (expand-file-name base-dir)) - nil))) - (let ((status (call-process - "git" - nil - `(:file, tmpfile-vc-head) - nil - "show" (concat "HEAD:" vc-file-name))) - (stderr (with-temp-buffer - (unless (zerop (cadr (insert-file-contents tmpfile-vc-head))) - (insert ": ")) - (buffer-substring-no-properties - (point-min) (line-end-position))))) - (when (stringp status) - (error "(git show HEAD:%s killed by signal %s%s)" - vc-file-name status stderr)) - (unless (zerop status) - (error "(git show HEAD:%s returned unsuccessfully %s%s)" - vc-file-name status stderr))))) - (t - (error - "Version control %s isn't supported, currently supported backends: git" - backend))))) + (with-temp-buffer + ;; We need to be able to find version control (git) root. + (unless base-dir + (error "File not known to version control system")) + (cond + ((string-equal backend "Git") + ;; Get the filename relative to git root. + (let ((vc-file-name (substring + (expand-file-name (buffer-file-name inp-buf)) + (string-width (expand-file-name base-dir)) + nil))) + (let ((status (call-process + "git" + nil + (current-buffer) + nil + "show" (concat "HEAD:" vc-file-name))) + (stderr (concat (if (zerop (buffer-size)) "" ": ") + (buffer-substring-no-properties + (point-min) (line-end-position))))) + (when (stringp status) + (error "(git show HEAD:%s killed by signal %s%s)" + vc-file-name status stderr)) + (unless (zerop status) + (error "(git show HEAD:%s returned unsuccessfully %s%s)" + vc-file-name status stderr))))) + (t + (error + "Version control %s isn't supported, currently supported backends: git" + backend))) + ;; Collect all lines where inp-buf (buffer we call + ;; clang-format-vc-diff on) differs from latest version of the + ;; backing file in the version control system. + (clang-format--vc-diff-compute-diff-and-get-lines + (current-buffer) inp-buf)))) (defun clang-format--region-impl (start end &optional style assume-file-name lines) @@ -323,7 +304,6 @@ specific locations for reformatting (i.e diff locations)." (delete-file temp-file) (when (buffer-name temp-buffer) (kill-buffer temp-buffer))))) - ;;;###autoload (defun clang-format-vc-diff (&optional style assume-file-name) "The same as ‘clang-format-buffer’ but only operates on the vc @@ -332,35 +312,15 @@ diffs from HEAD in the buffer. If no STYLE is given uses file. If no ASSUME-FILE-NAME is given uses the function ‘buffer-file-name’." (interactive) - (let ((tmpfile-vc-head nil) - (tmpfile-curbuf nil)) - (unwind-protect - (progn - (setq tmpfile-vc-head - (make-temp-file "clang-format-vc-tmp-head-content")) - (clang-format--vc-diff-get-vc-head-file tmpfile-vc-head) - ;; Move the current buffer to a temporary file to take a - ;; diff. Even if current-buffer is backed by a file, we - ;; want to diff the buffer contents which might not be - ;; saved. - (setq tmpfile-curbuf (make-temp-file "clang-format-vc-tmp")) - (write-region nil nil tmpfile-curbuf nil 'nomessage) - ;; Get a list of lines with a diff. - (let ((diff-lines - (clang-format--vc-diff-get-diff-lines - tmpfile-vc-head tmpfile-curbuf))) - ;; If we have any diffs, format them. - (when diff-lines - (clang-format--region-impl - (point-min) - (point-max) - style - assume-file-name - diff-lines)))) - (progn - ;; Cleanup temporary files we created. - (when tmpfile-vc-head (delete-file tmpfile-vc-head)) - (when tmpfile-curbuf (delete-file tmpfile-curbuf)))))) + (let ((diff-lines (clang-format--vc-diff-get-diff-lines))) + ;; If we have any diffs, format them. + (when diff-lines + (clang-format--region-impl + (point-min) + (point-max) + style + assume-file-name + diff-lines)))) ;;;###autoload >From b42785d10b744738f2aa67c7242a8a81439d53c5 Mon Sep 17 00:00:00 2001 From: Noah Goldstein <goldstein....@gmail.com> Date: Mon, 4 Nov 2024 14:54:05 -0600 Subject: [PATCH 06/12] Revert "use diff-no-select" This reverts commit fcf504f46ee3a46b9f6e8a21a25f07db2d4b215f. --- clang/tools/clang-format/clang-format.el | 168 ++++++++++++++--------- 1 file changed, 104 insertions(+), 64 deletions(-) diff --git a/clang/tools/clang-format/clang-format.el b/clang/tools/clang-format/clang-format.el index 0d732683d94f0a..f9ebede1b215c6 100644 --- a/clang/tools/clang-format/clang-format.el +++ b/clang/tools/clang-format/clang-format.el @@ -160,31 +160,55 @@ is a zero-based file offset, assuming ‘utf-8-unix’ coding." (string-to-number (match-string 3 line))))) (concat (match-string 1 line) ":" (match-string 1 line))))) -(defun clang-format--vc-diff-compute-diff-and-get-lines (buf-orig buf-cur) +(defun clang-format--vc-diff-get-diff-lines (file-orig file-new) "Return all line regions that contain diffs between FILE-ORIG and FILE-NEW. If there is no diff ‘nil’ is returned. Otherwise the return is a ‘list’ of lines in the format ‘--lines=<start>:<end>’ which can be passed directly to ‘clang-format’." ;; Use temporary buffer for output of diff. (with-temp-buffer - (diff-no-select buf-orig buf-cur "-a -U0" t (current-buffer)) - (let ((diff-lines '())) - ;; Iterate through all lines in diff buffer and collect all - ;; lines in current buffer that have a diff. - (goto-char (point-min)) - (while (not (eobp)) - (let ((diff-line (clang-format--vc-diff-match-diff-line + (let ((status (call-process + "diff" + nil + (current-buffer) + nil + ;; Binary diff has different behaviors that we + ;; aren't interested in. + "-a" + ;; Get minimal diff (copy diff config for git-clang-format). + "-U0" + file-orig + file-new)) + (stderr (concat (if (zerop (buffer-size)) "" ": ") (buffer-substring-no-properties - (line-beginning-position) - (line-end-position))))) - (when diff-line - ;; Create list line regions with diffs to pass to - ;; clang-format. - (push (concat "--lines=" diff-line) diff-lines))) - (forward-line 1)) - (reverse diff-lines)))) - -(defun clang-format--vc-diff-get-diff-lines () + (point-min) (line-end-position)))) + (diff-lines '())) + (cond + ((stringp status) + (error "(diff killed by signal %s%s)" status stderr)) + ;; Return of 0 indicates no diff. + ((= status 0) nil) + ;; Return of 1 indicates found diffs and no error. + ((= status 1) + ;; Iterate through all lines in diff buffer and collect all + ;; lines in current buffer that have a diff. + (goto-char (point-min)) + (while (not (eobp)) + (let ((diff-line (clang-format--vc-diff-match-diff-line + (buffer-substring-no-properties + (line-beginning-position) + (line-end-position))))) + (when diff-line + ;; Create list line regions with diffs to pass to + ;; clang-format. + (push (concat "--lines=" diff-line) diff-lines))) + (forward-line 1)) + (reverse diff-lines)) + ;; Any return != 0 && != 1 indicates some level of error. + (t + (error "(diff returned unsuccessfully %s%s)" status stderr)))))) + +(defun clang-format--vc-diff-get-vc-head-file (tmpfile-vc-head) "Stores the contents of ‘buffer-file-name’ at vc revision HEAD into ‘tmpfile-vc-head’. If the current buffer is either not a file or not in a vc repo, this results in an error. Currently git is the only @@ -193,44 +217,39 @@ supported vc." (unless (buffer-file-name) (error "Buffer is not visiting a file")) - (let ((inp-buf (current-buffer)) - (base-dir (vc-root-dir)) + (let ((base-dir (vc-root-dir)) (backend (vc-backend (buffer-file-name)))) - (with-temp-buffer - ;; We need to be able to find version control (git) root. - (unless base-dir - (error "File not known to version control system")) - (cond - ((string-equal backend "Git") - ;; Get the filename relative to git root. - (let ((vc-file-name (substring - (expand-file-name (buffer-file-name inp-buf)) - (string-width (expand-file-name base-dir)) - nil))) - (let ((status (call-process - "git" - nil - (current-buffer) - nil - "show" (concat "HEAD:" vc-file-name))) - (stderr (concat (if (zerop (buffer-size)) "" ": ") - (buffer-substring-no-properties - (point-min) (line-end-position))))) - (when (stringp status) - (error "(git show HEAD:%s killed by signal %s%s)" - vc-file-name status stderr)) - (unless (zerop status) - (error "(git show HEAD:%s returned unsuccessfully %s%s)" - vc-file-name status stderr))))) - (t - (error - "Version control %s isn't supported, currently supported backends: git" - backend))) - ;; Collect all lines where inp-buf (buffer we call - ;; clang-format-vc-diff on) differs from latest version of the - ;; backing file in the version control system. - (clang-format--vc-diff-compute-diff-and-get-lines - (current-buffer) inp-buf)))) + ;; We need to be able to find version control (git) root. + (unless base-dir + (error "File not known to git")) + (cond + ((string-equal backend "Git") + ;; Get the filename relative to git root. + (let ((vc-file-name (substring + (expand-file-name (buffer-file-name)) + (string-width (expand-file-name base-dir)) + nil))) + (let ((status (call-process + "git" + nil + `(:file, tmpfile-vc-head) + nil + "show" (concat "HEAD:" vc-file-name))) + (stderr (with-temp-buffer + (unless (zerop (cadr (insert-file-contents tmpfile-vc-head))) + (insert ": ")) + (buffer-substring-no-properties + (point-min) (line-end-position))))) + (when (stringp status) + (error "(git show HEAD:%s killed by signal %s%s)" + vc-file-name status stderr)) + (unless (zerop status) + (error "(git show HEAD:%s returned unsuccessfully %s%s)" + vc-file-name status stderr))))) + (t + (error + "Version control %s isn't supported, currently supported backends: git" + backend))))) (defun clang-format--region-impl (start end &optional style assume-file-name lines) @@ -304,6 +323,7 @@ specific locations for reformatting (i.e diff locations)." (delete-file temp-file) (when (buffer-name temp-buffer) (kill-buffer temp-buffer))))) + ;;;###autoload (defun clang-format-vc-diff (&optional style assume-file-name) "The same as ‘clang-format-buffer’ but only operates on the vc @@ -312,15 +332,35 @@ diffs from HEAD in the buffer. If no STYLE is given uses file. If no ASSUME-FILE-NAME is given uses the function ‘buffer-file-name’." (interactive) - (let ((diff-lines (clang-format--vc-diff-get-diff-lines))) - ;; If we have any diffs, format them. - (when diff-lines - (clang-format--region-impl - (point-min) - (point-max) - style - assume-file-name - diff-lines)))) + (let ((tmpfile-vc-head nil) + (tmpfile-curbuf nil)) + (unwind-protect + (progn + (setq tmpfile-vc-head + (make-temp-file "clang-format-vc-tmp-head-content")) + (clang-format--vc-diff-get-vc-head-file tmpfile-vc-head) + ;; Move the current buffer to a temporary file to take a + ;; diff. Even if current-buffer is backed by a file, we + ;; want to diff the buffer contents which might not be + ;; saved. + (setq tmpfile-curbuf (make-temp-file "clang-format-vc-tmp")) + (write-region nil nil tmpfile-curbuf nil 'nomessage) + ;; Get a list of lines with a diff. + (let ((diff-lines + (clang-format--vc-diff-get-diff-lines + tmpfile-vc-head tmpfile-curbuf))) + ;; If we have any diffs, format them. + (when diff-lines + (clang-format--region-impl + (point-min) + (point-max) + style + assume-file-name + diff-lines)))) + (progn + ;; Cleanup temporary files we created. + (when tmpfile-vc-head (delete-file tmpfile-vc-head)) + (when tmpfile-curbuf (delete-file tmpfile-curbuf)))))) ;;;###autoload >From 95db21cc5636e37ff6044c2894c2bd593fd335cb Mon Sep 17 00:00:00 2001 From: Noah Goldstein <goldstein....@gmail.com> Date: Mon, 4 Nov 2024 14:55:23 -0600 Subject: [PATCH 07/12] comment why no diff-no-select --- clang/tools/clang-format/clang-format.el | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang/tools/clang-format/clang-format.el b/clang/tools/clang-format/clang-format.el index f9ebede1b215c6..26fecc81645322 100644 --- a/clang/tools/clang-format/clang-format.el +++ b/clang/tools/clang-format/clang-format.el @@ -167,6 +167,9 @@ return is a ‘list’ of lines in the format ‘--lines=<start>:<end>’ which can be passed directly to ‘clang-format’." ;; Use temporary buffer for output of diff. (with-temp-buffer + ;; We could use diff.el:diff-no-select here. The reason we don't + ;; is diff-no-select requires extra copies on the buffers which + ;; induces noticable slowdowns, especially on larger files. (let ((status (call-process "diff" nil >From 98d5894b84e4c4ea28267ceb6cde368bddabc803 Mon Sep 17 00:00:00 2001 From: Noah Goldstein <goldstein....@gmail.com> Date: Mon, 4 Nov 2024 14:57:21 -0600 Subject: [PATCH 08/12] Use 'diff-command' from diff.el --- clang/tools/clang-format/clang-format.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/tools/clang-format/clang-format.el b/clang/tools/clang-format/clang-format.el index 26fecc81645322..43a0ad7c7d8586 100644 --- a/clang/tools/clang-format/clang-format.el +++ b/clang/tools/clang-format/clang-format.el @@ -171,7 +171,7 @@ which can be passed directly to ‘clang-format’." ;; is diff-no-select requires extra copies on the buffers which ;; induces noticable slowdowns, especially on larger files. (let ((status (call-process - "diff" + diff-command nil (current-buffer) nil >From 1e6524939388e0ef79c224a358e38076c5410bbd Mon Sep 17 00:00:00 2001 From: Noah Goldstein <goldstein....@gmail.com> Date: Mon, 4 Nov 2024 22:37:40 -0600 Subject: [PATCH 09/12] git-vc-program/ignore errors --- clang/tools/clang-format/clang-format.el | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/tools/clang-format/clang-format.el b/clang/tools/clang-format/clang-format.el index 43a0ad7c7d8586..9108d229ef5ff9 100644 --- a/clang/tools/clang-format/clang-format.el +++ b/clang/tools/clang-format/clang-format.el @@ -233,7 +233,7 @@ supported vc." (string-width (expand-file-name base-dir)) nil))) (let ((status (call-process - "git" + vc-git-program nil `(:file, tmpfile-vc-head) nil @@ -323,7 +323,7 @@ specific locations for reformatting (i.e diff locations)." (if incomplete-format (message "(clang-format: incomplete (syntax errors)%s)" stderr) (message "(clang-format: success%s)" stderr)))) - (delete-file temp-file) + (ignore-errors (delete-file temp-file)) (when (buffer-name temp-buffer) (kill-buffer temp-buffer))))) @@ -360,7 +360,7 @@ file. If no ASSUME-FILE-NAME is given uses the function style assume-file-name diff-lines)))) - (progn + (ignore-errors ;; Cleanup temporary files we created. (when tmpfile-vc-head (delete-file tmpfile-vc-head)) (when tmpfile-curbuf (delete-file tmpfile-curbuf)))))) >From 7cdfa4e106e9e34a108513b83396b601a06a7a8d Mon Sep 17 00:00:00 2001 From: Noah Goldstein <goldstein....@gmail.com> Date: Tue, 5 Nov 2024 08:45:27 -0600 Subject: [PATCH 10/12] Ignore errors deleting both --- clang/tools/clang-format/clang-format.el | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/tools/clang-format/clang-format.el b/clang/tools/clang-format/clang-format.el index 9108d229ef5ff9..149e44357b9ecf 100644 --- a/clang/tools/clang-format/clang-format.el +++ b/clang/tools/clang-format/clang-format.el @@ -360,10 +360,10 @@ file. If no ASSUME-FILE-NAME is given uses the function style assume-file-name diff-lines)))) - (ignore-errors + (progn ;; Cleanup temporary files we created. - (when tmpfile-vc-head (delete-file tmpfile-vc-head)) - (when tmpfile-curbuf (delete-file tmpfile-curbuf)))))) + (when tmpfile-vc-head (ignore-errors (delete-file tmpfile-vc-head))) + (when tmpfile-curbuf (ignore-errors (delete-file tmpfile-curbuf))))))) ;;;###autoload >From 6d49dcfb4934042754f96bf1bc6a9eb4438644d1 Mon Sep 17 00:00:00 2001 From: Noah Goldstein <goldstein....@gmail.com> Date: Tue, 5 Nov 2024 17:09:05 -0600 Subject: [PATCH 11/12] Nits/Diff command that doesn't req regex --- clang/tools/clang-format/clang-format.el | 51 ++++++++---------------- 1 file changed, 17 insertions(+), 34 deletions(-) diff --git a/clang/tools/clang-format/clang-format.el b/clang/tools/clang-format/clang-format.el index 149e44357b9ecf..df8845f799e45a 100644 --- a/clang/tools/clang-format/clang-format.el +++ b/clang/tools/clang-format/clang-format.el @@ -146,20 +146,6 @@ is a zero-based file offset, assuming ‘utf-8-unix’ coding." (lambda (byte &optional _quality _coding-system) (byte-to-position (1+ byte))))) -(defun clang-format--vc-diff-match-diff-line (line) - ;; We are matching something like: - ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@" - ;; Return as "<LineStart>:<LineEnd>" - (when (string-match "^@@\s-[0-9,]+\s\\+\\([0-9]+\\)\\(,\\([0-9]+\\)\\)?\s@@$" line) - ;; If we have multi-line diff - (if (match-string 3 line) - (concat (match-string 1 line) - ":" - (number-to-string - (+ (string-to-number (match-string 1 line)) - (string-to-number (match-string 3 line))))) - (concat (match-string 1 line) ":" (match-string 1 line))))) - (defun clang-format--vc-diff-get-diff-lines (file-orig file-new) "Return all line regions that contain diffs between FILE-ORIG and FILE-NEW. If there is no diff ‘nil’ is returned. Otherwise the @@ -169,7 +155,7 @@ which can be passed directly to ‘clang-format’." (with-temp-buffer ;; We could use diff.el:diff-no-select here. The reason we don't ;; is diff-no-select requires extra copies on the buffers which - ;; induces noticable slowdowns, especially on larger files. + ;; induces noticeable slowdowns, especially on larger files. (let ((status (call-process diff-command nil @@ -178,14 +164,16 @@ which can be passed directly to ‘clang-format’." ;; Binary diff has different behaviors that we ;; aren't interested in. "-a" - ;; Get minimal diff (copy diff config for git-clang-format). - "-U0" + ;; Print new lines in file-new formatted as + ;; "--lines=<StartDiff:EndDiff> " + "--changed-group-format=%(N=0?:--lines=%dF:%dM )" + ;; Don't print anything for unchanged lines + "--unchanged-group-format=" file-orig file-new)) (stderr (concat (if (zerop (buffer-size)) "" ": ") (buffer-substring-no-properties - (point-min) (line-end-position)))) - (diff-lines '())) + (point-min) (line-end-position))))) (cond ((stringp status) (error "(diff killed by signal %s%s)" status stderr)) @@ -193,20 +181,15 @@ which can be passed directly to ‘clang-format’." ((= status 0) nil) ;; Return of 1 indicates found diffs and no error. ((= status 1) - ;; Iterate through all lines in diff buffer and collect all - ;; lines in current buffer that have a diff. - (goto-char (point-min)) - (while (not (eobp)) - (let ((diff-line (clang-format--vc-diff-match-diff-line - (buffer-substring-no-properties - (line-beginning-position) - (line-end-position))))) - (when diff-line - ;; Create list line regions with diffs to pass to - ;; clang-format. - (push (concat "--lines=" diff-line) diff-lines))) - (forward-line 1)) - (reverse diff-lines)) + ;; We had our diff command printout all diffs as + ;; "--lines=S0:E0 --lines=S1:E1 ... --lines=SN:EN " so just + ;; split the output into a list to pass to clang-format. + (split-string + (buffer-substring-no-properties (point-min) (point-max)) + ;; All whitespace (practically only spaces). + "[ \f\t\n\r\v]" + ;; Don't create empty entries. + t)) ;; Any return != 0 && != 1 indicates some level of error. (t (error "(diff returned unsuccessfully %s%s)" status stderr)))))) @@ -235,7 +218,7 @@ supported vc." (let ((status (call-process vc-git-program nil - `(:file, tmpfile-vc-head) + `(:file ,tmpfile-vc-head) nil "show" (concat "HEAD:" vc-file-name))) (stderr (with-temp-buffer >From 7742bd1a0f63c1e21bf71ef39121e587c96f0846 Mon Sep 17 00:00:00 2001 From: Noah Goldstein <goldstein....@gmail.com> Date: Thu, 7 Nov 2024 11:23:42 -0600 Subject: [PATCH 12/12] Use macro for cleaning files --- clang/tools/clang-format/clang-format.el | 66 +++++++++++++----------- 1 file changed, 37 insertions(+), 29 deletions(-) diff --git a/clang/tools/clang-format/clang-format.el b/clang/tools/clang-format/clang-format.el index df8845f799e45a..78daa052ce85d5 100644 --- a/clang/tools/clang-format/clang-format.el +++ b/clang/tools/clang-format/clang-format.el @@ -146,6 +146,17 @@ is a zero-based file offset, assuming ‘utf-8-unix’ coding." (lambda (byte &optional _quality _coding-system) (byte-to-position (1+ byte))))) +(defmacro clang-format--with-delete-files-guard (bind-files-to-delete &rest body) + "Execute BODY which may add temp files to BIND-FILES-TO-DELETE." + (declare (indent 1)) + `(let ((,bind-files-to-delete nil)) + (unwind-protect + (progn + ,@body) + (while ,bind-files-to-delete + (with-demoted-errors "failed to remove file: %S" + (delete-file (pop ,bind-files-to-delete))))))) + (defun clang-format--vc-diff-get-diff-lines (file-orig file-new) "Return all line regions that contain diffs between FILE-ORIG and FILE-NEW. If there is no diff ‘nil’ is returned. Otherwise the @@ -318,35 +329,32 @@ diffs from HEAD in the buffer. If no STYLE is given uses file. If no ASSUME-FILE-NAME is given uses the function ‘buffer-file-name’." (interactive) - (let ((tmpfile-vc-head nil) - (tmpfile-curbuf nil)) - (unwind-protect - (progn - (setq tmpfile-vc-head - (make-temp-file "clang-format-vc-tmp-head-content")) - (clang-format--vc-diff-get-vc-head-file tmpfile-vc-head) - ;; Move the current buffer to a temporary file to take a - ;; diff. Even if current-buffer is backed by a file, we - ;; want to diff the buffer contents which might not be - ;; saved. - (setq tmpfile-curbuf (make-temp-file "clang-format-vc-tmp")) - (write-region nil nil tmpfile-curbuf nil 'nomessage) - ;; Get a list of lines with a diff. - (let ((diff-lines - (clang-format--vc-diff-get-diff-lines - tmpfile-vc-head tmpfile-curbuf))) - ;; If we have any diffs, format them. - (when diff-lines - (clang-format--region-impl - (point-min) - (point-max) - style - assume-file-name - diff-lines)))) - (progn - ;; Cleanup temporary files we created. - (when tmpfile-vc-head (ignore-errors (delete-file tmpfile-vc-head))) - (when tmpfile-curbuf (ignore-errors (delete-file tmpfile-curbuf))))))) + (clang-format--with-delete-files-guard tmp-files + (let ((tmpfile-vc-head nil) + (tmpfile-curbuf nil)) + (setq tmpfile-vc-head + (make-temp-file "clang-format-vc-tmp-head-content")) + (push tmpfile-vc-head tmp-files) + (clang-format--vc-diff-get-vc-head-file tmpfile-vc-head) + ;; Move the current buffer to a temporary file to take a + ;; diff. Even if current-buffer is backed by a file, we + ;; want to diff the buffer contents which might not be + ;; saved. + (setq tmpfile-curbuf (make-temp-file "clang-format-vc-tmp")) + (push tmpfile-curbuf tmp-files) + (write-region nil nil tmpfile-curbuf nil 'nomessage) + ;; Get a list of lines with a diff. + (let ((diff-lines + (clang-format--vc-diff-get-diff-lines + tmpfile-vc-head tmpfile-curbuf))) + ;; If we have any diffs, format them. + (when diff-lines + (clang-format--region-impl + (point-min) + (point-max) + style + assume-file-name + diff-lines)))))) ;;;###autoload _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits