[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Nov 3 08:36:31 PST 2024
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.w.n at gmail.com>
Date: Thu, 17 Oct 2024 17:31:24 -0500
Subject: [PATCH 1/3] [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.w.n at gmail.com>
Date: Sun, 3 Nov 2024 10:31:27 -0600
Subject: [PATCH 2/3] 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.w.n at gmail.com>
Date: Sun, 3 Nov 2024 10:36:16 -0600
Subject: [PATCH 3/3] 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))))))
More information about the cfe-commits
mailing list