[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[LLVM Continuous Integration via cfe-commits]

llvm-ci wrote:

LLVM Buildbot has detected a new failure on builder `llvm-clang-aarch64-darwin` 
running on `doug-worker-4` while building `clang` at step 6 
"test-build-unified-tree-check-all".

Full details are available at: 
https://lab.llvm.org/buildbot/#/builders/190/builds/14086


Here is the relevant piece of the build log for the reference

```
Step 6 (test-build-unified-tree-check-all) failure: test (failure)
 TEST 'Clang :: Analysis/live-stmts.cpp' FAILED 

Exit Code: 1

Command Output (stderr):
--
RUN: at line 1: /Users/buildbot/buildbot-root/aarch64-darwin/build/bin/clang 
-cc1 -internal-isystem 
/Users/buildbot/buildbot-root/aarch64-darwin/build/lib/clang/21/include 
-nostdsysteminc -analyze -analyzer-constraints=range -setup-static-analyzer -w 
-analyzer-checker=debug.DumpLiveExprs 
/Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang/test/Analysis/live-stmts.cpp
 2>&1   | /Users/buildbot/buildbot-root/aarch64-darwin/build/bin/FileCheck 
/Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang/test/Analysis/live-stmts.cpp
+ /Users/buildbot/buildbot-root/aarch64-darwin/build/bin/clang -cc1 
-internal-isystem 
/Users/buildbot/buildbot-root/aarch64-darwin/build/lib/clang/21/include 
-nostdsysteminc -analyze -analyzer-constraints=range -setup-static-analyzer -w 
-analyzer-checker=debug.DumpLiveExprs 
/Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang/test/Analysis/live-stmts.cpp
+ /Users/buildbot/buildbot-root/aarch64-darwin/build/bin/FileCheck 
/Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang/test/Analysis/live-stmts.cpp
/Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang/test/Analysis/live-stmts.cpp:239:16:
 error: CHECK-EMPTY: is not on the line after the previous 
match
// CHECK-EMPTY:
   ^
:180:1: note: 'next' match was here

^
:177:1: note: previous match ended here

^
:178:1: note: non-matching line after 
previous match is here
ImplicitCastExpr 0x11a81cd78 '_Bool' 
^

Input file: 
Check file: 
/Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/clang/test/Analysis/live-stmts.cpp

-dump-input=help explains the following input dump.

Input was:
<<
   1:  
   2: [ B0 (live expressions at block 
exit) ] 
check:21  ^~~
   3:  
empty:22  ^
   4:  
empty:23  ^
   5: [ B1 (live expressions at block 
exit) ] 
check:24  ^~~
   6:  
empty:25  ^
   7:  
empty:26  ^
   8: [ B2 (live expressions at block 
exit) ] 
check:27  ^~~
   9:  
empty:28  ^
  10: DeclRefExpr 0x10d85eee0 'int' 
lvalue ParmVar 0x10d841a70 'y' 'int' 
next:29   
^~
  11:  
empty:30  ^
  12: DeclRefExpr 0x10d85ef00 'int' 
lvalue ParmVar 0x10d841af0 'z' 'int' 
...

```



https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

https://github.com/goldsteinn closed 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 approved this pull request.


https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Luke Lau via cfe-commits]

lukel97 wrote:

Just a heads up I've been using this locally for a bit now and it's been great, 
thanks for working on this. Haven't run into any issues so far.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

goldsteinn wrote:

@ideasman42 ping

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

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 
Date: Thu, 17 Oct 2024 17:31:24 -0500
Subject: [PATCH 01/17] [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=:'
+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=:... --lines=:" 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-nam

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

goldsteinn wrote:

``

> Works well on Linux, generally seems fine although I still think it's worth 
> considering splitting out "getting-changes" into a separate shared code-base 
> to avoid dealing with details of calling diff for different version control 
> system here.

Think lukel97 also tested on MacOS.

I see where you're coming from regarding the split, although still feel quite 
strongly that features that don't make it into the mainline project have a high 
risk of becoming derelict.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 commented:

Works well on Linux, generally seems fine although I still think it's worth 
considering splitting out "getting-changes" into a separate shared code-base to 
avoid dealing with details of calling diff for different version control system 
here.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

@@ -146,18 +146,125 @@ 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
+(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
+return is a ‘list’ of lines in the format ‘--lines=:’
+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 noticeable slowdowns, especially on larger files.
+(let ((status (call-process
+   diff-command
+   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
+   (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)
+;; Find and collect all diff lines.
+;; We are matching something like:
+;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+(goto-char (point-min))
+(while (re-search-forward
+
"^@@[[:blank:]]-[[:digit:],]+[[:blank:]]\\+\\([[:digit:]]+\\)\\(,\\([[:digit:]]+\\)\\)?[[:blank:]]@@$"
+nil
+t
+1)
+  (let ((match1 (string-to-number (match-string 1)))
+(match3 (let ((match3_or_nil (match-string 3)))
+  (if match3_or_nil
+  (string-to-number match3_or_nil)
+nil
+(push (format
+   "--lines=%d:%d"
+   match1
+   (if match3 (+ match1 match3) match1))
+  diff-lines)))
+(nreverse diff-lines))
+   ;; Any return != 0 && != 1 indicates some level of error.
+   (t
+(error "(diff returned unsuccessfully %s%s)" status stderr))

ideasman42 wrote:

All errors should use the "clang-format: " prefix, otherwise it can be 
difficult to track down errors when these functions are called indirectly.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

@@ -146,18 +146,125 @@ 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
+(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)

ideasman42 wrote:

\*picky\* - personal preference, but I find it odd that the function returns 
command line arguments for clang-format.

It can return a list of cons-pairs of instead `(list (cons int int) ...)`  
instead, then the clang-format function can convert them to the arguments used 
by clang-format.

(the patch I attached did this).

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

@@ -216,17 +327,72 @@ uses the function `buffer-file-name'."
 (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))

ideasman42 wrote:

suggest with-demoted-errors here too, could be a function as it's used 
elsewhere.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

goldsteinn wrote:

> > Checking again and am still considering this patch too spesific/incomplete, 
> > checking vc's diff calls to git - they are considerably more involved than 
> > in this PR, meaning this PR will likely require follow up commits to fix 
> > problems _(see `vc-diff-internal`, inlined below for reference, it deals 
> > with EOL conversion, added files, coding systems... things this PR doesn't 
> > really handle)._
> 
> I'm not sure why the complexity of `vc-diff-internal` is really related to 
> the complexity. We only use `vc-root-dir` and `vc-backend`. Further, the `vc` 
> system is not some unstable new feature, its core since 25.1. I don't see why 
> we would expect this to fall over all of a sudden.
> 
> Can you expand on what is incomplete or too specific about it. We have 
> `git-clang-format`, this is essentially just porting that functionality to 
> emacs. Is the issue that we only support "git"? Or something else?
> 
> > Attached a patch that allows for formatting line-ranges, the line range 
> > generation must be implemented externally.
> > ```
> > * The `clang-format-modified-fn` customizable function is used to return a 
> > list of "modified" line ranges, this can be set by 3rd party packages - VC 
> > implementation independent.
> > 
> > * This function simply returns a  list of integer pairs (line ranges).
> > 
> > * An error is raised if the function isn't set.
> > ```
> 
> That seems far more specific and incomplete... but ultimately if its the only 
> part that can be accepted its better than nothing.
> 
> > Patch files:
> > ```
> > * Patch on the main branch
> >   
> > [pr-112792-update.diff.txt](https://github.com/user-attachments/files/18309256/pr-112792-update.diff.txt)
> > 
> > * The whole file (for convenience).
> >   
> > [clang-format.el.update.txt](https://github.com/user-attachments/files/18309260/clang-format.el.update.txt)
> > 
> > * The git/diff logic extracted into a separate file - which would not be 
> > applied to the LLVM project, just use for testing.
> >   
> > [clang-format-git-vc-diff.el.txt](https://github.com/user-attachments/files/18309263/clang-format-git-vc-diff.el.txt)
> > ```
> > 
> > 
> > 
> >   
> > 
> > 
> >   
> > 
> > 
> > 
> >   
> > Perhaps support for version control diff's can be supported by 
> > [clang-format-plus](https://github.com/SavchenkoValeriy/emacs-clang-format-plus).
> 
> Ultimately I really want to get this into actual clang-format where it will 
> be maintained (by myself included) and kept up to date.
> 
> I also think this functionality will be useful for other developers (lukel97 
> at least seemed to express he would find it useful). Having it in some 
> tertiary repo seems liable to have it end up 1) not much used and 2) in a 
> state of decay in not to long.

@ideasman42

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Luke Lau via cfe-commits]

https://github.com/lukel97 approved this pull request.

Apologies for the delay on this again. However I tried it out locally and it 
now seems to work on macOS, thanks for fixing that! 

I really can't speak much for the elisp, but we don't have many reviewers for 
the emacs stuff and this feature would be really handy to have.

So LGTM, if it needs more review I think it can be done post-commit :)

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

goldsteinn wrote:

> Checking again and am still considering this patch too spesific/incomplete, 
> checking vc's diff calls to git - they are considerably more involved than in 
> this PR, meaning this PR will likely require follow up commits to fix 
> problems _(see `vc-diff-internal`, inlined below for reference, it deals with 
> EOL conversion, added files, coding systems... things this PR doesn't really 
> handle)._

I'm not sure why the complexity of `vc-diff-internal` is really related to the 
complexity. We only use `vc-root-dir` and `vc-backend`. Further, the `vc` 
system is not some unstable new feature, its core since 25.1. I don't see why 
we would expect this to fall over all of a sudden.

Can you expand on what is incomplete or too specific about it. We have 
`git-clang-format`, this is essentially just porting that functionality to 
emacs. Is the issue that we only support "git"? Or something else?

> 
> Attached a patch that allows for formatting line-ranges, the line range 
> generation must be implemented externally.
> 
> * The `clang-format-modified-fn` customizable function is used to return 
> a list of "modified" line ranges, this can be set by 3rd party packages - VC 
> implementation independent.
> 
> * This function simply returns a  list of integer pairs (line ranges).
> 
> * An error is raised if the function isn't set.
>

That seems far more specific and incomplete... but ultimately if its the only 
part that can be accepted its better than nothing.
 
> 
> Patch files:
> 
> * Patch on the main branch
>   
> [pr-112792-update.diff.txt](https://github.com/user-attachments/files/18309256/pr-112792-update.diff.txt)
> 
> * The whole file (for convenience).
>   
> [clang-format.el.update.txt](https://github.com/user-attachments/files/18309260/clang-format.el.update.txt)
> 
> * The git/diff logic extracted into a separate file - which would not be 
> applied to the LLVM project, just use for testing.
>   
> [clang-format-git-vc-diff.el.txt](https://github.com/user-attachments/files/18309263/clang-format-git-vc-diff.el.txt)
> 
> 
> Perhaps support for version control diff's can be supported by 
> [clang-format-plus](https://github.com/SavchenkoValeriy/emacs-clang-format-plus).

Ultimately I really want to get this into actual clang-format where it will be 
maintained (by myself included) and kept up to date.

I also think this functionality will be useful for other developers (lukel97 at 
least seemed to express he would find it useful).
Having it in some tertiary repo seems liable to have it end up 1) not much used 
and 2) in a state of decay in not to long.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

ideasman42 wrote:

Checking again and am still considering this patch too spesific/incomplete, 
checking vc's diff calls to git - they are considerably more involved than in 
this PR, meaning this PR will likely require follow up commits to fix problems 
_(see `vc-diff-internal`, inlined below for reference, it deals with EOL 
conversion, added files, coding systems... things this PR doesn't really 
handle)._

Attached a patch that allows for formatting line-ranges, the line range 
generation must be implemented externally.

- The `clang-format-modified-fn` customizable function is used to return a list 
of "modified" line ranges, this can be set by 3rd party packages - VC 
implementation independent.
- This function simply returns a  list of integer pairs (line ranges).
- An error is raised if the function isn't set.

Patch files:

- Patch on the main branch
  
[pr-112792-update.diff.txt](https://github.com/user-attachments/files/18309256/pr-112792-update.diff.txt)
- The whole file (for convenience).
  
[clang-format.el.update.txt](https://github.com/user-attachments/files/18309260/clang-format.el.update.txt)

- The git/diff logic extracted into a separate file - which would not be 
applied to the LLVM project, just use for testing.
  
[clang-format-git-vc-diff.el.txt](https://github.com/user-attachments/files/18309263/clang-format-git-vc-diff.el.txt)

-

As mentioned earlier, calling diff can be quite involved if all corner cases 
are properly handled.
```
(defun vc-diff-internal (async vc-fileset rev1 rev2 &optional verbose buffer)
  "Report diffs between revisions REV1 and REV2 of a fileset in VC-FILESET.
ASYNC non-nil means run the backend's commands asynchronously if possible.
VC-FILESET should have the format described in `vc-deduce-fileset'.
Output goes to the buffer BUFFER, which defaults to *vc-diff*.
BUFFER, if non-nil, should be a buffer or a buffer name.
Return t if the buffer had changes, nil otherwise."
  (unless buffer
(setq buffer "*vc-diff*"))
  (let* ((files (cadr vc-fileset))
 (messages (cons (format "Finding changes in %s..."
 (vc-delistify files))
 (format "No changes between %s and %s"
 (or rev1 "working revision")
 (or rev2 "workfile"
 ;; Set coding system based on the first file.  It's a kluge,
 ;; but the only way to set it for each file included would
 ;; be to call the back end separately for each file.
 (coding-system-for-read
  ;; Force the EOL conversion to be -unix, in case the files
  ;; to be compared have DOS EOLs.  In that case, EOL
  ;; conversion will produce a patch file that will either
  ;; fail to apply, or will change the EOL format of some of
  ;; the lines in the patched file.
  (coding-system-change-eol-conversion
   (if files (vc-coding-system-for-diff (car files)) 'undecided)
   'unix))
 (orig-diff-buffer-clone
  (if revert-buffer-in-progress-p
  (clone-buffer
   (generate-new-buffer-name " *vc-diff-clone*") nil
;; On MS-Windows and MS-DOS, Diff is likely to produce DOS-style
;; EOLs, which will look ugly if (car files) happens to have Unix
;; EOLs.  But for Git, we must force Unix EOLs in the diffs, since
;; Git always produces Unix EOLs in the parts that didn't come
;; from the file, and wants to see any CR characters when applying
;; patches.
(if (and (memq system-type '(windows-nt ms-dos))
 (not (eq (car vc-fileset) 'Git)))
(setq coding-system-for-read
  (coding-system-change-eol-conversion coding-system-for-read
   'dos)))
(vc-setup-buffer buffer)
(message "%s" (car messages))
;; Many backends don't handle well the case of a file that has been
;; added but not yet committed to the repo (notably CVS and Subversion).
;; Do that work here so the backends don't have to futz with it.  --ESR
;;
;; Actually most backends (including CVS) have options to control the
;; behavior since which one is better depends on the user and on the
;; situation).  Worse yet: this code does not handle the case where
;; `file' is a directory which contains added files.
;; I made it conditional on vc-diff-added-files but it should probably
;; just be removed (or copied/moved to specific backends).  --Stef.
(when vc-diff-added-files
  (let ((filtered '())
process-file-side-effects)
(dolist (file files)
  (if (or (file-directory-p file)
  (not (string= (vc-working-revision file) "0")))
  (push file filtered)
;; This file is added but not yet committed;
;; there is no repository version to diff against.
(if (or rev1 rev2)
(error "No revision

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

goldsteinn wrote:

ping @ideasman42  or @lukel97 

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

goldsteinn wrote:

ping @ideasman42 

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

goldsteinn wrote:

ping

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

goldsteinn wrote:

ping

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

goldsteinn wrote:

> > All things considered, I think requiring `vc` is a lot less intrusive than 
> > essentially our own `vc` wrapper
> 
> Not sure what you mean exactly by a wrapper, if git/diff logic can be 
> abstracted away - that seems a net gain. If ugly/fragile logic is required to 
> do that - it's debatable and may not be worth it (all things considered).

Well the wrapped is to hide the ugly git/diff stuff.

> 
> Anyway, I was only mentioning that we could consider abstracting away the 
> logic - if it's not practical, there is no need to go into details discussing 
> it, although I did mail the emacs-devel mailing list to check if this might 
> be supported:

My preference would be the get this in as is.


https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

ideasman42 wrote:

> All things considered, I think requiring `vc` is a lot less intrusive than 
> essentially our own `vc` wrapper

Not sure what you mean exactly by a wrapper, if git/diff logic can be 
abstracted away - that seems a net gain.
If ugly/fragile logic is required to do that - OK, it's debatable.

Anyway, I was only mentioning that we could consider abstracting away the logic 
- if it's not practical, there is no need to go into details discussing it, 
although I did mail the emacs-devel mailing list to check if this might be 
supported: 



https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

goldsteinn wrote:

> > I don't disagree these are all potential pitfalls (and there are certainly 
> > more), I just don't see how having the diff code in a separate project 
> > ameliorates any of them. And as stated earlier, I think it in fact 
> > complicates them.
> 
> The issue of "complexity" is quite subjective, a customizable function that 
> returns line ranges doesn't strike me as a complex API that's likely to 
> break/change with the benefit of easily integrating other diffing methods.
> 
> From a user perspective it likely just means one extra package, possibly 
> setting a configuration value.
> 
> Or, to avoid this PR having to handle system & version-control spesific 
> details - we could consider calling `vc-diff` directly to generate the diff - 
> this abstracts away all the details of calling git & diff directly, 
> personally I would still rather keep the functionality separate - then this 
> can be easily integrated with `hl-diff` or other packages that already track 
> changed lines, removing the overhead of the current method used.

`vc-diff` AFAICT won't work because it operates on files not buffer contents. 
It would be a nice simplifcation, although doesn't really change the fact we 
depend on `vc`.

All things considered, I think requiring `vc` is a lot less intrusive than 
essentially our own `vc` wrapper

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

ideasman42 wrote:

> I don't disagree these are all potential pitfalls (and there are certainly 
> more), I just don't see how having the diff code in a separate project 
> ameliorates any of them. And as stated earlier, I think it in fact 
> complicates them.

>From a user perspective it likely just means one extra package,possibly 
>setting a configuration value.
In a sense this PR is incomplete in that it only supports git/diff on Unix like 
systems.

We could consider a change to the PR that makes it less implementation spesific:

Use `vc-diff` to generate the diff - this abstracts away all the details of 
calling git or diff directly, personally I would still rather keep the 
functionality separate - then this can be easily integrated with `hl-diff` or 
other packages that already track changed lines, removing the overhead of the 
current method used.



https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

goldsteinn wrote:


> > Overall, I'm pretty down on this. IMO, the vc/diff functionality is pretty 
> > specific to the use-case we have in clang-format.el and neither is complex 
> > enough to warrant or made more convenient by having in an independent 
> > package.
> 
> Fair enough - as noted, this is fairly opinionated - and I don't think your 
> "wrong" for holding the contrary position.
> 
> Keep in mind many people have been using `clang-format.el` for years without 
> this functionality, and will continue to do so. I disagree that this change 
> is simple though, it seems simple on face value - based on the number of 
> replies in the review so far, issues calling out to git & diff with their 
> different versions ... possible optimizations, possible errors when they fail 
> ... is in fact more complicated than may first seem.
> 
> There are some potential bugs that could bite us:
> 
> * How to handle git failing (if git doesn't know about the file... the 
> repository is in some unexpected state... the file could be in the middle of 
> resolving a conflict for e.g.).
> * What if git or diff considers the file to be a binary file.
> * The emacs buffers should use the encoding settings from the buffer that is 
> being edited... do they? (I'd need to double check - at a guess - they don't 
> seem to at the moment).
> 

I don't disagree these are all potential pitfalls (and there are certainly 
more), I just don't see how having the diff code in a separate project 
ameliorates any of them. And as stated earlier, I think it in fact complicates 
them.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

ideasman42 wrote:

> So IIUC, you are proposing a create an entirely seperate project for 
> "vc-diff-lines" or something like that then making that a dependency for for 
> `clang-format`? Is that correct?

I would not depend on it - instead, it would be loosely coupled - if 
`clang-format-vc-diff-function` was set, it would be used. Otherwise 
`clang-format-vc-diff` wouldn't work (reporting the function to calculate 
changed lines needs to be set).

---

> At least IMO, this is the other way around. Relying on external projects I 
> think typically creates a higher maintainer burden, particularly in this case 
> where I think LLVM would be the only user of the new package, so whenever 
> changes where needed for the diff/vc stuff, it would require coordination 
> with an external project.

This can happen for intricate API's - in this case - a single function that 
returns line-ranges: `(list (int . int) ...)` is a fairly minimal interface, so 
I don't see separating the code out as adding overhead/burden.



> Overall, I'm pretty down on this. IMO, the vc/diff functionality is pretty 
> specific to the use-case we have in clang-format.el and neither is complex 
> enough to warrant or made more convenient by having in an independent package.

Fair enough - as noted, this is fairly opinionated - and I don't think your 
"wrong" for holding the contrary position.

Keep in mind many people have been using `clang-format.el` for years without 
this functionality, and will continue to do so.
I disagree that this change is simple though, it seems simple on face value - 
based on the review - using git & diff with their different versions ... 
possible optimizations, possible errors when they fail ... is in fact more 
complicated than may first seem.

There are some potential bugs that could bite us:

- How to handle git failing (if git doesn't know about the file... has a 
corrupt repository... the file could be in the middle of resolving a conflict).
- What if git or diff considers the file to be a binary file.
- The emacs buffers should use the encoding settings from the buffer that is 
being edited... do they? (I'd need to double check - at a guess - they don't 
seem to at the moment).

This is not to attempt to shoot down the contribution, just to note that there 
are all sorts of cases where things can go wrong - where a feature like this 
turns out not to be all that simple and there are corner cases that need to be 
investigated/resolved.

As noted, this is my suggestion to support a customizable line-range generator 
so `clang-format.el can be kept minimal. I'll leave the decision to others.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

goldsteinn wrote:

> _Suggesting a fairly large change to this PR, although it's quite 
> opinionated._
> 
> If I were maintaining `clang-format.el` I'd split out the logic for 
> generating a list of changes into it's own package.
> 
> Exactly how this is done is not be so important... it could for example 
> support a custom function that generates a list of line number pairs.
> 
> `clang-format-vc-diff` could depend on a custom function which defaults to 
> nil, and would warn when the function wasn't set. e.g. `(defcustom 
> clang-format-vc-diff-function ...)`
> 
> The function can simply return an ordered list of integer pairs representing 
> lines.
> 
> Then there can be a package on MELPA `clang-format-diff` or similar and all 
> the issues with platform compatibility can be handled there without pull 
> requests to LLVM.
>

So IIUC, you are proposing a create an entirely seperate project for 
"vc-diff-lines" or something like that then making that a dependency for for 
`clang-format`? Is that correct?
 
> There are a few reasons this has benefits.
> 
> * The current PR misses `(require ...)` for, `vc` `vc-git` ... however 
> requiring this is unnecessary for users who wont use the functionality.
> 
> * Any bugs relating to details of different versions of git/diff/WIN32 
> compatibility can be handled without going via LLVM PR.
> 
> * Support for other version control or other ways of generating change 
> ranges can be handled even user-customized - integrated with other packages 
> that already track changes - such as `diff-hl` could be used.
> 
> * All the details of integrating diff-hl/other-version-control, platform 
> support (proper error messages if `diff` or `git` isn't found or encounters 
> some unknown encoding)... can be handled separately.
> 
> * LLVM's `clang-format.el` remains minimal, users who fully format their 
> source don't need to install the extra functionality.
> 
> * Less maintenance burden for LLVM.
> 

At least IMO, this is the other way around. Relying on external projects I 
think typically creates a higher maintainer burden, particularly in this case 
where I think LLVM would be the only user of the new package, so whenever 
changes where needed for the diff/vc stuff, it would require coordination with 
an external project.

Further, if the external project got its own users and `clang-format.el`'s 
needs changed, it could become a lot more difficult to make the changes, as now 
there are independent users who might expect stability/etc...

> 
> The main down side is users who rely on this behavior need to install an 
> additional package although I don't see this as a big down-side.

Overall, I'm pretty down on this. IMO, the vc/diff functionality is pretty 
specific to the use-case we have in `clang-format.el` and neither is complex 
enough to warrant or made more convenient by having in an independent package.

I think a similiar argument would imply the `git-clang-format` script we 
already maintain under LLVM should be extract out (or at least all the commands 
that call into "git") which seems equally unappealing.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

ideasman42 wrote:

_Suggesting a fairly large change to this PR, although it's quite opinionated._

If I were maintaining `clang-format.el` I'd split out the logic for generating 
a list of changes into it's own package.

Exactly how this is done is not be so important... it could for example support 
a custom function that generates a list of line number pairs.

`clang-format-vc-diff` could depend on a custom function which defaults to nil, 
and would warn when the function wasn't set. e.g. `(defcustom 
clang-format-vc-diff-function ...)`

The function can simply return an ordered list of integer pairs representing  
lines.

Then there can be a package on MELPA `clang-format-diff`  or similar and all 
the issues with platform compatibility can be handled there without pull 
requests to LLVM.

---

There are a few reasons this has benefits.

- The current PR misses `(require ...)` for, `vc` `vc-git` ... however 
requiring this is unnecessary for users who wont use the functionality.
- Any bugs relating to details of different versions of git/diff/WIN32 
compatibility can be handled without going via LLVM PR.
- Support for other version control or other ways of generating change ranges 
can be handled even customized - integrated with other packages that already 
track changes - such as `diff-hl` could be used.
- All the details of integrating diff-hl/non-git version control, platform 
support (proper error messages if `diff` isn't found or encounters some unknown 
encoding)... can be handled separately.
- LLVM's clang-format.el remains minimal, users who fully format their source 
don't need to install the extra functionality.
- Less maintenance burden for LLVM.

The main down side is users who rely on this behavior need to install an 
additional package.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

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 
Date: Thu, 17 Oct 2024 17:31:24 -0500
Subject: [PATCH 01/16] [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=:'
+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=:... --lines=:" 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-nam

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

@@ -146,18 +146,124 @@ 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
+(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
+return is a ‘list’ of lines in the format ‘--lines=:’
+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 noticeable slowdowns, especially on larger files.
+(let ((status (call-process
+   diff-command
+   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
+   (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)
+;; Find and collect all diff lines.
+;; We are matching something like:
+;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+(goto-char (point-min))
+(while (re-search-forward
+"^@@\s-[0-9,]+\s\\+\\([0-9]+\\)\\(,\\([0-9]+\\)\\)?\s@@$"
+nil
+t
+1)
+  (let ((match1 (string-to-number (match-string 1)))
+(match3 (if (match-string 3)

ideasman42 wrote:

Minor point, suggest to use the result of the first `(match-string 3)` call 
instead of throwing the result away & calling again.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

@@ -146,18 +146,124 @@ 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
+(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
+return is a ‘list’ of lines in the format ‘--lines=:’
+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 noticeable slowdowns, especially on larger files.
+(let ((status (call-process
+   diff-command
+   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
+   (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)
+;; Find and collect all diff lines.
+;; We are matching something like:
+;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+(goto-char (point-min))
+(while (re-search-forward
+"^@@\s-[0-9,]+\s\\+\\([0-9]+\\)\\(,\\([0-9]+\\)\\)?\s@@$"
+nil
+t
+1)
+  (let ((match1 (string-to-number (match-string 1)))
+(match3 (if (match-string 3)

ideasman42 wrote:

`\s-` uses the syntax-table, in this case I think it's reasonable to be more 
explicit (space-or-tab)

This is a similar regex I used for the package `diff-at-point`.

Not saying you \*should\* use this, but in general I find `[:blank:]` 
preferable when `a space or tab character` is appropriate, see: 
https://www.emacswiki.org/emacs/RegularExpression
``
(concat
  "^\\(@@\\)[[:blank:]]+"
  ;; Previous (ignore).
  "-"
  
"\\([[:digit:]]+\\),\\([[:digit:]]+\\)"
  "[[:blank:]]+"
  ;; Current (use).
  "\\+"
  
"\\([[:digit:]]+\\),\\([[:digit:]]+\\)"
  "[[:blank:]]+@@")
```

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

@@ -206,20 +191,30 @@ 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))
+;; Find and collect all diff lines.
+;; We are matching something like:
+;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+(let ((diff-lines-re
+   "^@@\s-[0-9,]+\s\\+\\([0-9]+\\)\\(,\\([0-9]+\\)\\)?\s@@$")
+  (index 0)
+  (all-lines
+   (buffer-substring-no-properties (point-min) (point-max

goldsteinn wrote:

Fixed

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

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 
Date: Thu, 17 Oct 2024 17:31:24 -0500
Subject: [PATCH 01/15] [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=:'
+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=:... --lines=:" 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-nam

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

@@ -206,20 +191,30 @@ 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))
+;; Find and collect all diff lines.
+;; We are matching something like:
+;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+(let ((diff-lines-re
+   "^@@\s-[0-9,]+\s\\+\\([0-9]+\\)\\(,\\([0-9]+\\)\\)?\s@@$")
+  (index 0)
+  (all-lines
+   (buffer-substring-no-properties (point-min) (point-max

ideasman42 wrote:

Did you try searching within the current buffer?

It's quite strange to make a copy of the whole buffer just to search it.

Even if you can measure some performance improvement, I think it's reasonable 
to use common practices, instead of relying on performance characteristics of 
the current implementation. Unless there are really significant user visible 
advantages. Based on my own experience with ELisp, I've never seen scripts 
duplicate a buffer into a string to search it. Besides relying on some details 
of the performance of your version of emacs, subtle changes to ELisp may render 
this odd way of searching the buffer invalid. There is also the down side of 
storing the buffer twice in memory - probably it's not going to cause problems 
on modern systems, but it could cause some GC/performance issues with very 
large buffers still.

Furthermore `(while (re-search-forward ...) ...)` is such a common pattern, any 
bottleneck here could be caused by the arguments your using ... or some other 
buffer setting (case sensitivity.. or other arguments that control behavior). 
It seems quite suspicious that there should be a significant difference.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

@@ -146,18 +146,121 @@ 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--vc-diff-match-diff-line (line)
+  ;; We are matching something like:
+  ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+  ;; Return as ":"
+  (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
+return is a ‘list’ of lines in the format ‘--lines=:’
+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-command
+   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
+   (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))

goldsteinn wrote:

Update to essentially this (although by hand with `string-match`) as that was 
about 50% faster. I think because we drop string-properties but not really 
certain why.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

@@ -146,18 +146,104 @@ 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--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=:’
+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 noticeable slowdowns, especially on larger files.
+(let ((status (call-process
+   diff-command
+   nil
+   (current-buffer)
+   nil
+   ;; Binary diff has different behaviors that we
+   ;; aren't interested in.
+   "-a"
+   ;; Print new lines in file-new formatted as
+   ;; "--lines= "
+   "--changed-group-format=%(N=0?:--lines=%dF:%dM )"
+   ;; Don't print anything for unchanged lines
+   "--unchanged-group-format="

goldsteinn wrote:

Using the U0 only for now, can revisit GNU diff optimization later IMO.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

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 
Date: Thu, 17 Oct 2024 17:31:24 -0500
Subject: [PATCH 01/14] [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=:'
+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=:... --lines=:" 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-nam

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

@@ -146,18 +146,121 @@ 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--vc-diff-match-diff-line (line)
+  ;; We are matching something like:
+  ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+  ;; Return as ":"
+  (when (string-match 
"^@@\s-[0-9,]+\s\\+\\([0-9]+\\)\\(,\\([0-9]+\\)\\)?\s@@$" line)

goldsteinn wrote:

change `\s` -> `[:blank:]` makes the regex fail.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

@@ -146,18 +146,104 @@ 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--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=:’
+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 noticeable slowdowns, especially on larger files.
+(let ((status (call-process
+   diff-command
+   nil
+   (current-buffer)
+   nil
+   ;; Binary diff has different behaviors that we
+   ;; aren't interested in.
+   "-a"
+   ;; Print new lines in file-new formatted as
+   ;; "--lines= "
+   "--changed-group-format=%(N=0?:--lines=%dF:%dM )"
+   ;; Don't print anything for unchanged lines
+   "--unchanged-group-format="

ideasman42 wrote:

Suggest to use diff compatible with BSD/Linux - it _could_ be worth detecting 
which diff is in use and using `--unchanged-group-format` as an optimization, 
but that could be applied as an optimization later.

Or, I suppose it could be a custom setting which only defaults to true on Linux.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

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 
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=:'
+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=:... --lines=:" 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-nam

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

@@ -146,18 +146,104 @@ 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--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=:’
+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 noticeable slowdowns, especially on larger files.
+(let ((status (call-process
+   diff-command
+   nil
+   (current-buffer)
+   nil
+   ;; Binary diff has different behaviors that we
+   ;; aren't interested in.
+   "-a"
+   ;; Print new lines in file-new formatted as
+   ;; "--lines= "
+   "--changed-group-format=%(N=0?:--lines=%dF:%dM )"
+   ;; Don't print anything for unchanged lines
+   "--unchanged-group-format="

goldsteinn wrote:

Its not fundementally needed, but it speeds it up as we can then skip scanning 
all the diffs.
Does bsd diff support `--unchanged-line-format`?

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

@@ -146,18 +146,104 @@ 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--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=:’
+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 noticeable slowdowns, especially on larger files.
+(let ((status (call-process
+   diff-command
+   nil
+   (current-buffer)
+   nil
+   ;; Binary diff has different behaviors that we
+   ;; aren't interested in.
+   "-a"
+   ;; Print new lines in file-new formatted as
+   ;; "--lines= "
+   "--changed-group-format=%(N=0?:--lines=%dF:%dM )"
+   ;; Don't print anything for unchanged lines
+   "--unchanged-group-format="

goldsteinn wrote:

Although maybe we should just stick with the `-U0` option, it works and is 
probably more universally supported.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Luke Lau via cfe-commits]

@@ -146,18 +146,104 @@ 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--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=:’
+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 noticeable slowdowns, especially on larger files.
+(let ((status (call-process
+   diff-command
+   nil
+   (current-buffer)
+   nil
+   ;; Binary diff has different behaviors that we
+   ;; aren't interested in.
+   "-a"
+   ;; Print new lines in file-new formatted as
+   ;; "--lines= "
+   "--changed-group-format=%(N=0?:--lines=%dF:%dM )"
+   ;; Don't print anything for unchanged lines
+   "--unchanged-group-format="

lukel97 wrote:

Hey sorry for the delay, I'm not very well versed in elisp unfortunately. But 
this looks really useful and I would definitely use it.

I tried this out locally though on macOS and it looks like BSD diff doesn't 
have `--unchanged-group-format`. Is this needed for the rest of it to work? 
This isn't a blocking comment btw, we can always tell people to just install 
diffutils.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

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 
Date: Thu, 17 Oct 2024 17:31:24 -0500
Subject: [PATCH 01/10] [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=:'
+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=:... --lines=:" 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-nam

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

@@ -312,15 +335,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

ideasman42 wrote:

\*picky\* I find it often works better to wrap `unwind-protect` in a macro.

Example usage 
```
(clang-format--with-delete-files-guard files-to-delete
  ... snip ...
(push (setq tmpfile-curbuf (make-temp-file "clang-format-vc-tmp")) 
temp-files)
  (push tmpfile-curbuf temp-files))
```k

Here is a macro that does this:
```
(defmacro clang-format--with-advice (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)))
```


https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

@@ -146,18 +146,118 @@ 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--vc-diff-match-diff-line (line)
+  ;; We are matching something like:
+  ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+  ;; Return as ":"
+  (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
+return is a ‘list’ of lines in the format ‘--lines=:’
+which can be passed directly to ‘clang-format’."
+  ;; Use temporary buffer for output of diff.
+  (with-temp-buffer
+(let ((status (call-process
+   "diff"

goldsteinn wrote:

Done :)

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

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 
Date: Thu, 17 Oct 2024 17:31:24 -0500
Subject: [PATCH 1/8] [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=:'
+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=:... --lines=:" 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)

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

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 
Date: Thu, 17 Oct 2024 17:31:24 -0500
Subject: [PATCH 1/7] [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=:'
+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=:... --lines=:" 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)

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

@@ -146,18 +146,115 @@ 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--vc-diff-match-diff-line (line)
+  ;; Matching something like:
+  ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+  ;; Return as ":"
+  (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
+return is a 'list' of lines in the format '--lines=:'
+which can be passed directly to 'clang-format'"
+  ;; Temporary buffer for output of diff.
+  (with-temp-buffer
+(let ((status (call-process

ideasman42 wrote:

In this case, include a code-comment for why `diff-no-select` is not used.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

@@ -146,18 +146,115 @@ 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--vc-diff-match-diff-line (line)
+  ;; Matching something like:
+  ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+  ;; Return as ":"
+  (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
+return is a 'list' of lines in the format '--lines=:'
+which can be passed directly to 'clang-format'"
+  ;; Temporary buffer for output of diff.
+  (with-temp-buffer
+(let ((status (call-process

goldsteinn wrote:

Also note even if we stick with temporary file, `diff-no-select` still 
re-copies the file.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

@@ -146,18 +146,115 @@ 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--vc-diff-match-diff-line (line)
+  ;; Matching something like:
+  ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+  ;; Return as ":"
+  (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
+return is a 'list' of lines in the format '--lines=:'
+which can be passed directly to 'clang-format'"
+  ;; Temporary buffer for output of diff.
+  (with-temp-buffer
+(let ((status (call-process

goldsteinn wrote:

NB: These times are with no lines going to clang-format, the call to 
clang-format itself is dominant (depends on actual code, but on ValueTracking 
~5x the cost of the diff setup).

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

@@ -146,18 +146,115 @@ 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--vc-diff-match-diff-line (line)
+  ;; Matching something like:
+  ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+  ;; Return as ":"
+  (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
+return is a 'list' of lines in the format '--lines=:'
+which can be passed directly to 'clang-format'"
+  ;; Temporary buffer for output of diff.
+  (with-temp-buffer
+(let ((status (call-process

goldsteinn wrote:

NB: Current commit is with `diff-no-select`. I didn't update comments yet as 
want to see if we are going to stick with the impl first.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

@@ -146,18 +146,115 @@ 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--vc-diff-match-diff-line (line)
+  ;; Matching something like:
+  ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+  ;; Return as ":"
+  (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
+return is a 'list' of lines in the format '--lines=:'
+which can be passed directly to 'clang-format'"
+  ;; Temporary buffer for output of diff.
+  (with-temp-buffer
+(let ((status (call-process

goldsteinn wrote:

Okay, I'm opposed to changing to use `diff-no-select`. It is notably slower. I 
think because we go through temporary buffers. I'm going to push the code that 
uses `diff-no-select`, unless you see a way to speed it up I think we should 
keep the current implementation.

Benchmark code:
```
(require 'benchmark)
(benchmark-elapse
  (with-current-buffer "ValueTracking.cpp" ;; 1 line file
(clang-format-vc-diff)
(clang-format-vc-diff)
(clang-format-vc-diff)
(clang-format-vc-diff)
(clang-format-vc-diff)
(clang-format-vc-diff)
(clang-format-vc-diff)
(clang-format-vc-diff)
(clang-format-vc-diff)
(clang-format-vc-diff)
)
  )
```
Bespoke Diff   : 0.1 += 0.05 sec
`diff-no-select`: 0.2 += 0.05 sec

If I scale `ValueTracking.cpp` to 10 lines:

Bespoke Diff   : 0.45 += 0.1 sec
`diff-no-select`: 1.1 += 0.1 sec

So it seems twice as slow, I imagine because we end up sending the VC version 
of the file through memory.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

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 
Date: Thu, 17 Oct 2024 17:31:24 -0500
Subject: [PATCH 1/5] [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=:'
+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=:... --lines=:" 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)

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

@@ -146,18 +146,115 @@ 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--vc-diff-match-diff-line (line)
+  ;; Matching something like:
+  ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+  ;; Return as ":"
+  (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
+return is a 'list' of lines in the format '--lines=:'
+which can be passed directly to 'clang-format'"
+  ;; Temporary buffer for output of diff.
+  (with-temp-buffer
+(let ((status (call-process

goldsteinn wrote:

So initially it was because I was using the format modifiers for `diff` output 
as direct input to `clang-format`. `diff-no-select` adds some output. Since I 
switches to just use the same method as `git-clang-format`, we can switch to 
use `diff-no-select`. 


https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

@@ -146,18 +146,118 @@ 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--vc-diff-match-diff-line (line)
+  ;; We are matching something like:
+  ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+  ;; Return as ":"
+  (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
+return is a ‘list’ of lines in the format ‘--lines=:’
+which can be passed directly to ‘clang-format’."
+  ;; Use temporary buffer for output of diff.
+  (with-temp-buffer
+(let ((status (call-process
+   "diff"

ideasman42 wrote:

While I think it may be best to call `diff` functions directly, if this is call 
is kept, it's probably best to use `diff-command`, so the diff command used by 
`vc/diff.el` is used here, it may be necessary on WIN32.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

@@ -146,18 +146,115 @@ 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--vc-diff-match-diff-line (line)
+  ;; Matching something like:
+  ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+  ;; Return as ":"
+  (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
+return is a 'list' of lines in the format '--lines=:'
+which can be passed directly to 'clang-format'"
+  ;; Temporary buffer for output of diff.
+  (with-temp-buffer
+(let ((status (call-process

ideasman42 wrote:

Ah, I wasn't aware of that, even so, there is some advantage in using emacs 
diff functionality as it handles error cases, removing temp files etc.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

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 
Date: Thu, 17 Oct 2024 17:31:24 -0500
Subject: [PATCH 1/4] [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=:'
+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=:... --lines=:" 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)

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

@@ -146,18 +146,115 @@ 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--vc-diff-match-diff-line (line)
+  ;; Matching something like:
+  ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+  ;; Return as ":"
+  (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
+return is a 'list' of lines in the format '--lines=:'
+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"
+   ;; 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
+   (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
+  (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--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"))
+  ;; Only version control currently supported is Git
+  (unless (string-equal (vc-backend (buffer-file-name)) "Git")

goldsteinn wrote:

Sure, the counter argument is it causes a bit of excessive nesting, but happy 
to change.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

@@ -146,18 +146,115 @@ 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--vc-diff-match-diff-line (line)
+  ;; Matching something like:
+  ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+  ;; Return as ":"
+  (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
+return is a 'list' of lines in the format '--lines=:'
+which can be passed directly to 'clang-format'"
+  ;; Temporary buffer for output of diff.
+  (with-temp-buffer
+(let ((status (call-process

goldsteinn wrote:

I don't think so, or at least `diff.el` uses temporary files:

```
(defun diff-file-local-copy (file-or-buf)
  "Like `file-local-copy' but also supports a buffer as the argument.
When FILE-OR-BUF is a buffer, return the filename of a local
temporary file with the buffer's contents."
  (if (bufferp file-or-buf)
  (with-current-buffer file-or-buf
(let ((tempfile (make-temp-file "buffer-content-")))
  (if diff-entire-buffers
  (write-region nil nil tempfile nil 'nomessage)
(write-region (point-min) (point-max) tempfile nil 'nomessage))
  tempfile))
(file-local-copy file-or-buf)))
...
  (let* ((old-alt (diff-file-local-copy old))
 (new-alt (diff-file-local-copy new))
 (command
  (mapconcat #'identity
 `(,diff-command
   ;; Use explicitly specified switches
   ,@switches
   ,@(mapcar #'shell-quote-argument
 (nconc
  (and (or old-alt new-alt)

```

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

@@ -146,18 +146,115 @@ 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--vc-diff-match-diff-line (line)
+  ;; Matching something like:
+  ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+  ;; Return as ":"
+  (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
+return is a 'list' of lines in the format '--lines=:'
+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"
+   ;; 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
+   (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
+  (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--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"))
+  ;; 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

ideasman42 wrote:

\*picky\* use full sentences for comments - end with a full-stop, applies to 
many other comments here.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

@@ -146,18 +146,115 @@ 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--vc-diff-match-diff-line (line)
+  ;; Matching something like:
+  ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+  ;; Return as ":"
+  (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

ideasman42 wrote:

Running `byte-compile-file` gives various warnings: 

```
clang-format.el:163:2: Warning: docstring has wrong usage of unescaped single 
quotes (use \=' or different quoting such as `...')
clang-format.el:201:19: Error: ‘add-to-list’ can’t use lexical var 
‘diff-lines’; use ‘push’ or ‘cl-pushnew’

In clang-format--vc-diff-get-vc-head-file:
clang-format.el:211:2: Warning: docstring has wrong usage of unescaped single 
quotes (use \=' or different quoting such as `...')

In clang-format--region-impl:
clang-format.el:252:2: Warning: docstring has wrong usage of unescaped single 
quotes (use \=' or different quoting such as `...')

In clang-format-vc-diff:
clang-format.el:325:2: Warning: docstring has wrong usage of unescaped single 
quotes (use \=' or different quoting such as `...')
```


https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 requested changes to this pull request.

This seems more ore less OK, although 

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

@@ -146,18 +146,115 @@ 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--vc-diff-match-diff-line (line)
+  ;; Matching something like:
+  ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+  ;; Return as ":"
+  (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
+return is a 'list' of lines in the format '--lines=:'
+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"
+   ;; 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
+   (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
+  (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--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"))
+  ;; Only version control currently supported is Git
+  (unless (string-equal (vc-backend (buffer-file-name)) "Git")

ideasman42 wrote:

Suggestion: my preference would be to write this in a way that makes adding 
other VC more straightforward.
```
(let ((vc-backend ...snip...))
  (cond
(string-equal vc-backend "Git")
  ...git...logic...
(t
   (error "Version control %s isn't supported, currently supported backends 
... snip ..." vc-backend
```

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

@@ -146,18 +146,115 @@ 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--vc-diff-match-diff-line (line)
+  ;; Matching something like:
+  ;; "@@ -80 +80 @@" or "@@ -80,2 +80,2 @@"
+  ;; Return as ":"
+  (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
+return is a 'list' of lines in the format '--lines=:'
+which can be passed directly to 'clang-format'"
+  ;; Temporary buffer for output of diff.
+  (with-temp-buffer
+(let ((status (call-process

ideasman42 wrote:

Regarding temporary file use: did you check on the `diff-buffers` function? it 
should be possible to diff two temporary buffers without creating temporary 
files.

Looking into the implementation `diff-no-select` may be what your after as that 
looks to be intended for non interactive use.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

goldsteinn wrote:

Current return from `package-lint`:

```
3 issues found:

1:0: error: Package should have a Homepage or URL header.
1:60: warning: You should depend on (emacs "24.1") if you need lexical-binding.
223:19: error: You should depend on (emacs "25.1") if you need `vc-root-dir'.
```

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

https://github.com/goldsteinn edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

https://github.com/goldsteinn edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

goldsteinn wrote:

NB: The new API is 'clang-format-vc-diff'. I will update commit title when 
merging.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

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 
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=:'
+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=:... --lines=:" 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)

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

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 
Date: Thu, 17 Oct 2024 17:31:24 -0500
Subject: [PATCH 1/2] [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=:'
+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=:... --lines=:" 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)

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

@@ -205,14 +288,60 @@ 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)

goldsteinn wrote:

The reason I chose `diffs` was to indicate it wasn't just the diff at point or 
something, but Ill rename `clang-format-vc-diff`

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

@@ -132,18 +132,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=:'
+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=:... --lines=:" 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")))

goldsteinn wrote:

`with-temp-file` doesn't quite work here, V2 I just wrap the thing in an 
`unwind-protect` and delete the temp files in it.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Luke Lau via cfe-commits]

@@ -205,14 +288,60 @@ 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)

lukel97 wrote:

I think the term diff would be more consistent with existing emacs functions 
e.g. vc-diff, diff-delete-trailing-whitespace etc. 

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[via cfe-commits]

goldsteinn wrote:

> Note that I'm not a regular clang contributor, I just submitted a small 
> improvement to `clang-format.el`, and maintain some emacs packages in elpa & 
> melpa.
> 
> Overall the PR looks like it needs more attention to detail, as far as I can 
> tell it's creating temporary files and never removing them, various minor 
> issues noted inline.
> 
> * This PR needs to be rebased on top of the recently added 
> `clang-format-on-save-mode` commit.
> 
> * Running `package-lint` reports.
> 
> 
> ```
> 156:19: warning: Closing parens should not be wrapped onto new lines.
> 157:18: warning: Closing parens should not be wrapped onto new lines.
> 176:12: error: You should depend on (emacs "24.4") or the compat package if 
> you need `string-trim'.
> 188:11: error: You should depend on (emacs "25.1") if you need `vc-root-dir'.
> 198:59: error: You should depend on (emacs "25.1") if you need `vc-root-dir'.
> ```

Thank you for the detailed feedback, I will have v2 up tomorrow.

Regarding the package lints, is requiring 25.1 acceptable? I'm not really sure 
how to implement this without `vc-root-dir`.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

@@ -205,14 +288,60 @@ 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)

ideasman42 wrote:

A more general point, would it make sense to call this `clang-format-vc-diffs` 
- which currently only supports git?

This way we wouldn't need `clang-format-hg-diffs` or separate commands to 
support other version control systems in the future.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

@@ -205,14 +288,60 @@ 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")))

ideasman42 wrote:

It looks like this temporary file isn't removed either.

https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

[Campbell Barton via cfe-commits]

https://github.com/ideasman42 edited 
https://github.com/llvm/llvm-project/pull/112792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits