[PATCH] D37903: Fix assume-filename handling in clang-format.el
This revision was automatically updated to reflect the committed changes.
Closed by commit rC319621: Fix assume-filename handling in clang-format.el
(authored by phst).
Changed prior to commit:
https://reviews.llvm.org/D37903?vs=124242=125274#toc
Repository:
rC Clang
https://reviews.llvm.org/D37903
Files:
tools/clang-format/clang-format.el
Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -119,18 +119,23 @@
(byte-to-position (1+ byte)
;;;###autoload
-(defun clang-format-region (start end style)
+(defun clang-format-region (start end 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 active region. If no style is given uses `clang-format-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
(unless style
(setq style clang-format-style))
+ (unless assume-file-name
+(setq assume-file-name buffer-file-name))
+
(let ((file-start (clang-format--bufferpos-to-filepos start 'approximate
'utf-8-unix))
(file-end (clang-format--bufferpos-to-filepos end 'approximate
@@ -144,16 +149,21 @@
;; always use ‘utf-8-unix’ and ignore the buffer coding system.
(default-process-coding-system '(utf-8-unix . utf-8-unix)))
(unwind-protect
-(let ((status (call-process-region
- nil nil clang-format-executable
- nil `(,temp-buffer ,temp-file) nil
-
- "-output-replacements-xml"
- "-assume-filename" (or (buffer-file-name) "")
- "-style" style
- "-offset" (number-to-string file-start)
- "-length" (number-to-string (- file-end file-start))
- "-cursor" (number-to-string cursor)))
+(let ((status (apply #'call-process-region
+ nil nil clang-format-executable
+ nil `(,temp-buffer ,temp-file) nil
+ `("-output-replacements-xml"
+ ;; Gaurd against a nil assume-file-name.
+ ;; If the clang-format option -assume-filename
+ ;; is given a blank string it will crash as per
+ ;; the following bug report
+ ;; https://bugs.llvm.org/show_bug.cgi?id=34667
+ ,@(and assume-file-name
+ (list "-assume-filename"
assume-file-name))
+ "-style" ,style
+ "-offset" ,(number-to-string file-start)
+ "-length" ,(number-to-string (- file-end
file-start))
+ "-cursor" ,(number-to-string cursor
(stderr (with-temp-buffer
(unless (zerop (cadr (insert-file-contents temp-file)))
(insert ": "))
@@ -181,10 +191,13 @@
(when (buffer-name temp-buffer) (kill-buffer temp-buffer)
;;;###autoload
-(defun clang-format-buffer ( style)
- "Use clang-format to format the current buffer according to STYLE."
+(defun clang-format-buffer ( style assume-file-name)
+ "Use clang-format to format the current buffer according to STYLE.
+If no STYLE is given uses `clang-format-style'. Use ASSUME-FILE-NAME
+to locate a style config file. If no ASSUME-FILE-NAME is given uses
+the function `buffer-file-name'."
(interactive)
- (clang-format-region (point-min) (point-max) style))
+ (clang-format-region (point-min) (point-max) style assume-file-name))
;;;###autoload
(defalias 'clang-format 'clang-format-region)
Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -119,18 +119,23 @@
(byte-to-position (1+ byte)
;;;###autoload
-(defun clang-format-region (start end style)
+(defun clang-format-region (start end 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 active region. If no style is given uses `clang-format-style'."
+If called interactively uses the region or the
[PATCH] D37903: Fix assume-filename handling in clang-format.el
werbitt updated this revision to Diff 124242.
werbitt edited the summary of this revision.
werbitt added a comment.
Updating for upstream head
https://reviews.llvm.org/D37903
Files:
tools/clang-format/clang-format.el
Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -119,18 +119,23 @@
(byte-to-position (1+ byte)
;;;###autoload
-(defun clang-format-region (start end style)
+(defun clang-format-region (start end 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 active region. If no style is given uses `clang-format-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
(unless style
(setq style clang-format-style))
+ (unless assume-file-name
+(setq assume-file-name buffer-file-name))
+
(let ((file-start (clang-format--bufferpos-to-filepos start 'approximate
'utf-8-unix))
(file-end (clang-format--bufferpos-to-filepos end 'approximate
@@ -144,16 +149,21 @@
;; always use ‘utf-8-unix’ and ignore the buffer coding system.
(default-process-coding-system '(utf-8-unix . utf-8-unix)))
(unwind-protect
-(let ((status (call-process-region
- nil nil clang-format-executable
- nil `(,temp-buffer ,temp-file) nil
-
- "-output-replacements-xml"
- "-assume-filename" (or (buffer-file-name) "")
- "-style" style
- "-offset" (number-to-string file-start)
- "-length" (number-to-string (- file-end file-start))
- "-cursor" (number-to-string cursor)))
+(let ((status (apply #'call-process-region
+ nil nil clang-format-executable
+ nil `(,temp-buffer ,temp-file) nil
+ `("-output-replacements-xml"
+ ;; Gaurd against a nil assume-file-name.
+ ;; If the clang-format option -assume-filename
+ ;; is given a blank string it will crash as per
+ ;; the following bug report
+ ;; https://bugs.llvm.org/show_bug.cgi?id=34667
+ ,@(and assume-file-name
+ (list "-assume-filename"
assume-file-name))
+ "-style" ,style
+ "-offset" ,(number-to-string file-start)
+ "-length" ,(number-to-string (- file-end
file-start))
+ "-cursor" ,(number-to-string cursor
(stderr (with-temp-buffer
(unless (zerop (cadr (insert-file-contents temp-file)))
(insert ": "))
@@ -181,10 +191,13 @@
(when (buffer-name temp-buffer) (kill-buffer temp-buffer)
;;;###autoload
-(defun clang-format-buffer ( style)
- "Use clang-format to format the current buffer according to STYLE."
+(defun clang-format-buffer ( style assume-file-name)
+ "Use clang-format to format the current buffer according to STYLE.
+If no STYLE is given uses `clang-format-style'. Use ASSUME-FILE-NAME
+to locate a style config file. If no ASSUME-FILE-NAME is given uses
+the function `buffer-file-name'."
(interactive)
- (clang-format-region (point-min) (point-max) style))
+ (clang-format-region (point-min) (point-max) style assume-file-name))
;;;###autoload
(defalias 'clang-format 'clang-format-region)
Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -119,18 +119,23 @@
(byte-to-position (1+ byte)
;;;###autoload
-(defun clang-format-region (start end style)
+(defun clang-format-region (start end 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 active region. If no style is given uses `clang-format-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
[PATCH] D37903: Fix assume-filename handling in clang-format.el
phi added a comment. The patch doesn't apply any more to upstream head: $ arc patch D37903 patching file tools/clang-format/clang-format.el Hunk #1 FAILED at 122. Hunk #2 FAILED at 193. Could you please rebase it? https://reviews.llvm.org/D37903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37903: Fix assume-filename handling in clang-format.el
phi added a comment. Hi, do you need anything more from us? This patch looks fine and can be submitted. https://reviews.llvm.org/D37903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37903: Fix assume-filename handling in clang-format.el
werbitt updated this revision to Diff 117574. werbitt added a comment. Use uppercase when referring to arguments https://reviews.llvm.org/D37903 Files: tools/clang-format/clang-format.el Index: tools/clang-format/clang-format.el === --- tools/clang-format/clang-format.el +++ tools/clang-format/clang-format.el @@ -122,8 +122,8 @@ (defun clang-format-region (start end 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 +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) @@ -193,8 +193,8 @@ ;;;###autoload (defun clang-format-buffer ( style assume-file-name) "Use clang-format to format the current buffer according to STYLE. -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 +If no STYLE is given uses `clang-format-style'. Use ASSUME-FILE-NAME +to locate a style config file. If no ASSUME-FILE-NAME is given uses the function `buffer-file-name'." (interactive) (clang-format-region (point-min) (point-max) style assume-file-name)) Index: tools/clang-format/clang-format.el === --- tools/clang-format/clang-format.el +++ tools/clang-format/clang-format.el @@ -122,8 +122,8 @@ (defun clang-format-region (start end 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 +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) @@ -193,8 +193,8 @@ ;;;###autoload (defun clang-format-buffer ( style assume-file-name) "Use clang-format to format the current buffer according to STYLE. -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 +If no STYLE is given uses `clang-format-style'. Use ASSUME-FILE-NAME +to locate a style config file. If no ASSUME-FILE-NAME is given uses the function `buffer-file-name'." (interactive) (clang-format-region (point-min) (point-max) style assume-file-name)) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37903: Fix assume-filename handling in clang-format.el
phst accepted this revision. phst added inline comments. Comment at: tools/clang-format/clang-format.el:126 +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'." nit: write arguments in docstrings always in caps: ASSUME-FILE-NAME Comment at: tools/clang-format/clang-format.el:197 +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'." Write the argument in the docstring as ASSUME-FILE-NAME https://reviews.llvm.org/D37903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37903: Fix assume-filename handling in clang-format.el
werbitt updated this revision to Diff 117500.
werbitt added a comment.
Thanks, I was too hasty when I tried to fix the mess I created when I diffed
against an updated master and uploaded it without noticing. I believe this diff
gets me back to where I intended.
https://reviews.llvm.org/D37903
Files:
tools/clang-format/clang-format.el
Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -120,11 +120,11 @@
;;;###autoload
(defun clang-format-region (start end style assume-file-name)
- "Use clang-format to format the code between START and END according to STYLE
-using ASSUME-FILE-NAME to locate a style config file. If called interactively
-uses the region or the current statement if there is no active region. If no
-style is given uses `clang-format-style'. If no assume-file-name is given uses
-`buffer-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))
@@ -154,8 +154,9 @@
nil `(,temp-buffer ,temp-file) nil
`("-output-replacements-xml"
;; Gaurd against a nil assume-file-name.
- ;; If -assume-filename is given a blank string
- ;; it will crash as per the following bug report
+ ;; If the clang-format option -assume-filename
+ ;; is given a blank string it will crash as per
+ ;; the following bug report
;; https://bugs.llvm.org/show_bug.cgi?id=34667
,@(and assume-file-name
(list "-assume-filename"
assume-file-name))
@@ -191,9 +192,10 @@
;;;###autoload
(defun clang-format-buffer ( style assume-file-name)
- "Use clang-format to format the current buffer according to STYLE using
-ASSUME-FILE-NAME to locate a style config file. If no style is given uses
-`clang-format-style'. If no assume-file-name is given uses `buffer-file-name'."
+ "Use clang-format to format the current buffer according to STYLE.
+If no style is given uses `clang-format-style'. Use ASSUME-FILE-NAME
+to locate a style config file. If no assume-file-name is given uses
+the function `buffer-file-name'."
(interactive)
(clang-format-region (point-min) (point-max) style assume-file-name))
Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -120,11 +120,11 @@
;;;###autoload
(defun clang-format-region (start end style assume-file-name)
- "Use clang-format to format the code between START and END according to STYLE
-using ASSUME-FILE-NAME to locate a style config file. If called interactively
-uses the region or the current statement if there is no active region. If no
-style is given uses `clang-format-style'. If no assume-file-name is given uses
-`buffer-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))
@@ -154,8 +154,9 @@
nil `(,temp-buffer ,temp-file) nil
`("-output-replacements-xml"
;; Gaurd against a nil assume-file-name.
- ;; If -assume-filename is given a blank string
- ;; it will crash as per the following bug report
+ ;; If the clang-format option -assume-filename
+ ;; is given a blank string it will crash as per
+ ;; the following bug report
;; https://bugs.llvm.org/show_bug.cgi?id=34667
,@(and assume-file-name
(list "-assume-filename" assume-file-name))
@@ -191,9 +192,10 @@
;;;###autoload
(defun clang-format-buffer ( style assume-file-name)
- "Use clang-format to format the current buffer according to STYLE using
-ASSUME-FILE-NAME to locate a style config
[PATCH] D37903: Fix assume-filename handling in clang-format.el
phst added inline comments. Comment at: tools/clang-format/clang-format.el:122 ;;;###autoload -(defun clang-format-region (start end style assume-file-name) - "Use clang-format to format the code between START and END according to STYLE -using ASSUME-FILE-NAME to locate a style config file. If called interactively -uses the region or the current statement if there is no active region. If no -style is given uses `clang-format-style'. If no assume-file-name is given uses -`buffer-file-name'." +(defun clang-format-region (start end style) + "Use clang-format to format the code between START and END according to STYLE. Hmm, somehow your patch got reverted? https://reviews.llvm.org/D37903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37903: Fix assume-filename handling in clang-format.el
werbitt updated this revision to Diff 116605.
werbitt added a comment.
Cleanup Docs
- The first line of of an emacs docstring should be a complete sentence.
-Specify that "-assume-filname" is a clang-format option
https://reviews.llvm.org/D37903
Files:
tools/clang-format/clang-format.el
Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -119,12 +119,10 @@
(byte-to-position (1+ byte)
;;;###autoload
-(defun clang-format-region (start end style assume-file-name)
- "Use clang-format to format the code between START and END according to STYLE
-using ASSUME-FILE-NAME to locate a style config file. If called interactively
-uses the region or the current statement if there is no active region. If no
-style is given uses `clang-format-style'. If no assume-file-name is given uses
-`buffer-file-name'."
+(defun clang-format-region (start end style)
+ "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 active region. If no style is given uses `clang-format-style'."
(interactive
(if (use-region-p)
(list (region-beginning) (region-end))
@@ -133,9 +131,6 @@
(unless style
(setq style clang-format-style))
- (unless assume-file-name
-(setq assume-file-name buffer-file-name))
-
(let ((file-start (clang-format--bufferpos-to-filepos start 'approximate
'utf-8-unix))
(file-end (clang-format--bufferpos-to-filepos end 'approximate
@@ -149,20 +144,16 @@
;; always use ‘utf-8-unix’ and ignore the buffer coding system.
(default-process-coding-system '(utf-8-unix . utf-8-unix)))
(unwind-protect
-(let ((status (apply #'call-process-region
- nil nil clang-format-executable
- nil `(,temp-buffer ,temp-file) nil
- `("-output-replacements-xml"
- ;; Gaurd against a nil assume-file-name.
- ;; If -assume-filename is given a blank string
- ;; it will crash as per the following bug report
- ;; https://bugs.llvm.org/show_bug.cgi?id=34667
- ,@(and assume-file-name
- (list "-assume-filename"
assume-file-name))
- "-style" ,style
- "-offset" ,(number-to-string file-start)
- "-length" ,(number-to-string (- file-end
file-start))
- "-cursor" ,(number-to-string cursor
+(let ((status (call-process-region
+ nil nil clang-format-executable
+ nil `(,temp-buffer ,temp-file) nil
+
+ "-output-replacements-xml"
+ "-assume-filename" (or (buffer-file-name) "")
+ "-style" style
+ "-offset" (number-to-string file-start)
+ "-length" (number-to-string (- file-end file-start))
+ "-cursor" (number-to-string cursor)))
(stderr (with-temp-buffer
(unless (zerop (cadr (insert-file-contents temp-file)))
(insert ": "))
@@ -190,12 +181,10 @@
(when (buffer-name temp-buffer) (kill-buffer temp-buffer)
;;;###autoload
-(defun clang-format-buffer ( style assume-file-name)
- "Use clang-format to format the current buffer according to STYLE using
-ASSUME-FILE-NAME to locate a style config file. If no style is given uses
-`clang-format-style'. If no assume-file-name is given uses `buffer-file-name'."
+(defun clang-format-buffer ( style)
+ "Use clang-format to format the current buffer according to STYLE."
(interactive)
- (clang-format-region (point-min) (point-max) style assume-file-name))
+ (clang-format-region (point-min) (point-max) style))
;;;###autoload
(defalias 'clang-format 'clang-format-region)
Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -119,12 +119,10 @@
(byte-to-position (1+ byte)
;;;###autoload
-(defun clang-format-region (start end style assume-file-name)
- "Use clang-format to format the code between START and END according to STYLE
-using ASSUME-FILE-NAME to locate a style config file. If called interactively
-uses the region or the current statement if there is no active region. If no
-style is given uses `clang-format-style'. If no assume-file-name is given uses
-`buffer-file-name'."
+(defun clang-format-region (start end style)
+ "Use
[PATCH] D37903: Fix assume-filename handling in clang-format.el
werbitt reclaimed this revision. werbitt added a comment. This revision is now accepted and ready to land. It looks like I just uploaded a diff against a bad branch, and I can't figure out how to undo it. Sorry about that. https://reviews.llvm.org/D37903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37903: Fix assume-filename handling in clang-format.el
phst added inline comments. Comment at: tools/clang-format/clang-format.el:123 +(defun clang-format-region (start end style assume-file-name) + "Use clang-format to format the code between START and END according to STYLE +using ASSUME-FILE-NAME to locate a style config file. If called interactively Please stick to the canonical format, i.e. the first line should be a complete sentence. See https://www.gnu.org/software/emacs/manual/html_node/elisp/Documentation-Tips.html. You can use M-x checkdoc to detect such style issues automatically. Here I'd just leave the docstring intact and add another sentence describing ASSUME-FILE-NAME at the end. Comment at: tools/clang-format/clang-format.el:157 + ;; Gaurd against a nil assume-file-name. + ;; If -assume-filename is given a blank string + ;; it will crash as per the following bug report nit: "-assume-file-name" Comment at: tools/clang-format/clang-format.el:194 +(defun clang-format-buffer ( style assume-file-name) + "Use clang-format to format the current buffer according to STYLE using +ASSUME-FILE-NAME to locate a style config file. If no style is given uses Same here, please make the first line a complete sentence. https://reviews.llvm.org/D37903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37903: Fix assume-filename handling in clang-format.el
werbitt updated this revision to Diff 115875.
werbitt edited the summary of this revision.
werbitt added a comment.
Rename assume-file to assume-file-name and update documentation
https://reviews.llvm.org/D37903
Files:
tools/clang-format/clang-format.el
Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -119,20 +119,22 @@
(byte-to-position (1+ byte)
;;;###autoload
-(defun clang-format-region (start end style assume-file)
- "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 active region. If no style is given uses `clang-format-style'."
+(defun clang-format-region (start end style assume-file-name)
+ "Use clang-format to format the code between START and END according to STYLE
+using ASSUME-FILE-NAME to locate a style config file. If called interactively
+uses the region or the current statement if there is no active region. If no
+style is given uses `clang-format-style'. If no assume-file-name is given uses
+`buffer-file-name'."
(interactive
(if (use-region-p)
(list (region-beginning) (region-end))
(list (point) (point
(unless style
(setq style clang-format-style))
- (unless assume-file
-(setq assume-file buffer-file-name))
+ (unless assume-file-name
+(setq assume-file-name buffer-file-name))
(let ((file-start (clang-format--bufferpos-to-filepos start 'approximate
'utf-8-unix))
@@ -151,7 +153,12 @@
nil nil clang-format-executable
nil `(,temp-buffer ,temp-file) nil
`("-output-replacements-xml"
- ,@(and assume-file (list "-assume-filename"
assume-file))
+ ;; Gaurd against a nil assume-file-name.
+ ;; If -assume-filename is given a blank string
+ ;; it will crash as per the following bug report
+ ;; https://bugs.llvm.org/show_bug.cgi?id=34667
+ ,@(and assume-file-name
+ (list "-assume-filename"
assume-file-name))
"-style" ,style
"-offset" ,(number-to-string file-start)
"-length" ,(number-to-string (- file-end
file-start))
@@ -183,10 +190,12 @@
(when (buffer-name temp-buffer) (kill-buffer temp-buffer)
;;;###autoload
-(defun clang-format-buffer ( style assume-file)
- "Use clang-format to format the current buffer according to STYLE."
+(defun clang-format-buffer ( style assume-file-name)
+ "Use clang-format to format the current buffer according to STYLE using
+ASSUME-FILE-NAME to locate a style config file. If no style is given uses
+`clang-format-style'. If no assume-file-name is given uses `buffer-file-name'."
(interactive)
- (clang-format-region (point-min) (point-max) style assume-file))
+ (clang-format-region (point-min) (point-max) style assume-file-name))
;;;###autoload
(defalias 'clang-format 'clang-format-region)
Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -119,20 +119,22 @@
(byte-to-position (1+ byte)
;;;###autoload
-(defun clang-format-region (start end style assume-file)
- "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 active region. If no style is given uses `clang-format-style'."
+(defun clang-format-region (start end style assume-file-name)
+ "Use clang-format to format the code between START and END according to STYLE
+using ASSUME-FILE-NAME to locate a style config file. If called interactively
+uses the region or the current statement if there is no active region. If no
+style is given uses `clang-format-style'. If no assume-file-name is given uses
+`buffer-file-name'."
(interactive
(if (use-region-p)
(list (region-beginning) (region-end))
(list (point) (point
(unless style
(setq style clang-format-style))
- (unless assume-file
-(setq assume-file buffer-file-name))
+ (unless assume-file-name
+(setq assume-file-name buffer-file-name))
(let ((file-start (clang-format--bufferpos-to-filepos start 'approximate
'utf-8-unix))
@@ -151,7 +153,12 @@
nil nil clang-format-executable
nil `(,temp-buffer ,temp-file) nil
[PATCH] D37903: Fix assume-filename handling in clang-format.el
phst accepted this revision.
phst added a comment.
This revision is now accepted and ready to land.
Your use case sounds good to me. Please be sure to document the new parameter,
though.
Comment at: tools/clang-format/clang-format.el:154
+ `("-output-replacements-xml"
+ ,@(and assume-file (list "-assume-filename"
assume-file))
+ "-style" ,style
Please add a comment referencing the LLVM bug you've just filed so that others
aren't surprised about this.
https://reviews.llvm.org/D37903
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37903: Fix assume-filename handling in clang-format.el
phst added inline comments. Comment at: tools/clang-format/clang-format.el:125 If called interactively uses the region or the current statement if there is no active region. If no style is given uses `clang-format-style'." (interactive Please document the ASSUME-FILE parameter. Comment at: tools/clang-format/clang-format.el:187 ;;;###autoload -(defun clang-format-buffer ( style) +(defun clang-format-buffer ( style assume-file) "Use clang-format to format the current buffer according to STYLE." Nit: consider renaming the parameter to ASSUME-FILENAME so that it's clear that it's a filename. Same above. Comment at: tools/clang-format/clang-format.el:188 +(defun clang-format-buffer ( style assume-file) "Use clang-format to format the current buffer according to STYLE." (interactive) Please document the new parameter here as well. https://reviews.llvm.org/D37903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37903: Fix assume-filename handling in clang-format.el
werbitt added a comment. Hi, Thank you very much for your feedback. I submitted a bug here: https://bugs.llvm.org/show_bug.cgi?id=34667 I made the changes you suggested, but I left the assume-filename optional argument for now. My current use-case is that when I'm editing a source block in an orgmode file, the code is in new buffer that doesn't have a buffer-file-name. If I'm able to pass in the base orgmode buffer-file-name, I can apply the .clang-format file from the correct directory. Without it, I'll need to locate the .clang-format file and use it to populate the style argument, which would be a lot of extra code on my end. If you still think it's not worth it to have the assume-file argument let me know and I'll remove it. Thanks, Micah https://reviews.llvm.org/D37903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37903: Fix assume-filename handling in clang-format.el
werbitt updated this revision to Diff 115827.
werbitt added a comment.
Clean up call-process-region
- Quote call-process-region with #', this will cause a compile time error if
call-process-region is undefined
- Pass positional arguments normally (exclude from the list)
- Instead of using append use a backquoted list
- Use ,@ to splice the assume file logic into the list
- Use 'and' instead of 'if'
https://reviews.llvm.org/D37903
Files:
tools/clang-format/clang-format.el
Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -147,16 +147,15 @@
;; always use ‘utf-8-unix’ and ignore the buffer coding system.
(default-process-coding-system '(utf-8-unix . utf-8-unix)))
(unwind-protect
-(let ((status (apply 'call-process-region
- (append `(nil nil ,clang-format-executable
- nil (,temp-buffer ,temp-file) nil)
- '("-output-replacements-xml")
- (if assume-file
- `("-assume-filename" ,assume-file)
nil)
- `("-style" ,style
- "-offset" ,(number-to-string file-start)
- "-length" ,(number-to-string (-
file-end file-start))
- "-cursor" ,(number-to-string cursor)
+(let ((status (apply #'call-process-region
+ nil nil clang-format-executable
+ nil `(,temp-buffer ,temp-file) nil
+ `("-output-replacements-xml"
+ ,@(and assume-file (list "-assume-filename"
assume-file))
+ "-style" ,style
+ "-offset" ,(number-to-string file-start)
+ "-length" ,(number-to-string (- file-end
file-start))
+ "-cursor" ,(number-to-string cursor
(stderr (with-temp-buffer
(unless (zerop (cadr (insert-file-contents temp-file)))
(insert ": "))
Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -147,16 +147,15 @@
;; always use ‘utf-8-unix’ and ignore the buffer coding system.
(default-process-coding-system '(utf-8-unix . utf-8-unix)))
(unwind-protect
-(let ((status (apply 'call-process-region
- (append `(nil nil ,clang-format-executable
- nil (,temp-buffer ,temp-file) nil)
- '("-output-replacements-xml")
- (if assume-file
- `("-assume-filename" ,assume-file) nil)
- `("-style" ,style
- "-offset" ,(number-to-string file-start)
- "-length" ,(number-to-string (- file-end file-start))
- "-cursor" ,(number-to-string cursor)
+(let ((status (apply #'call-process-region
+ nil nil clang-format-executable
+ nil `(,temp-buffer ,temp-file) nil
+ `("-output-replacements-xml"
+ ,@(and assume-file (list "-assume-filename" assume-file))
+ "-style" ,style
+ "-offset" ,(number-to-string file-start)
+ "-length" ,(number-to-string (- file-end file-start))
+ "-cursor" ,(number-to-string cursor
(stderr (with-temp-buffer
(unless (zerop (cadr (insert-file-contents temp-file)))
(insert ": "))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37903: Fix assume-filename handling in clang-format.el
phst added a comment.
Thanks, generally looks good, but a couple of notes:
- This is actually a bug in clang-format, which can easily be triggered with
`clang-format -output-replacements-xml -assume-filename '' -offset 0 -length 10
-cursor 5 <<< ' = 1234;'`
It's fine to work around bugs, but please file a bug against clang-format
itself and reference it in a comment.
- Not sure whether the `assume-filename` parameter is actually needed. If
there's no known use case, please don't add it ("YAGNI").
- Prefer quoting function symbols with `#'`: `#'call-process-region`. The
byte-compiler will then warn if such a symbol is not defined as a function.
- It's more readable to write the non-varying arguments to call-process-region
as direct arguments to `apply`, like so: `(apply #'call-process-region nil ...
nil (append '("-output-replacements ...`. The result is equivalent, but this
structure makes it more obvious which arguments are positional and which are
rest arguments.
- You can combine the various lists with backquotes and get rid of `append`:
``("-output-replacements-xml" ,@(and buffer-file-name (list (concat
"-assume-filename=" buffer-file-name))) "-style" ...)`
https://reviews.llvm.org/D37903
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37903: Fix assume-filename handling in clang-format.el
klimek added a reviewer: phst. klimek added a comment. +Philipp, who is our emacs expert :) https://reviews.llvm.org/D37903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37903: Fix assume-filename handling in clang-format.el
werbitt created this revision.
When 'buffer-file-name' is nil 'call-process-region' returned a segmentation
fault error.
This was a problem when using clang-format-buffer on an orgmode source code
editing buffer.
I fixed this problem by excluding the '-assume-filename' argument when
'buffer-file-name' is nil.
To make it a bit more flexible I also added an optional argument,
'assume-file', to specify an assume-filename that overrides 'buffer-file-name'.
https://reviews.llvm.org/D37903
Files:
tools/clang-format/clang-format.el
Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -119,7 +119,7 @@
(byte-to-position (1+ byte)
;;;###autoload
-(defun clang-format-region (start end style)
+(defun clang-format-region (start end style assume-file)
"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 active region. If no style is given uses `clang-format-style'."
@@ -131,6 +131,9 @@
(unless style
(setq style clang-format-style))
+ (unless assume-file
+(setq assume-file buffer-file-name))
+
(let ((file-start (clang-format--bufferpos-to-filepos start 'approximate
'utf-8-unix))
(file-end (clang-format--bufferpos-to-filepos end 'approximate
@@ -144,16 +147,16 @@
;; always use ‘utf-8-unix’ and ignore the buffer coding system.
(default-process-coding-system '(utf-8-unix . utf-8-unix)))
(unwind-protect
-(let ((status (call-process-region
- nil nil clang-format-executable
- nil `(,temp-buffer ,temp-file) nil
-
- "-output-replacements-xml"
- "-assume-filename" (or (buffer-file-name) "")
- "-style" style
- "-offset" (number-to-string file-start)
- "-length" (number-to-string (- file-end file-start))
- "-cursor" (number-to-string cursor)))
+(let ((status (apply 'call-process-region
+ (append `(nil nil ,clang-format-executable
+ nil (,temp-buffer ,temp-file) nil)
+ '("-output-replacements-xml")
+ (if assume-file
+ `("-assume-filename" ,assume-file)
nil)
+ `("-style" ,style
+ "-offset" ,(number-to-string file-start)
+ "-length" ,(number-to-string (-
file-end file-start))
+ "-cursor" ,(number-to-string cursor)
(stderr (with-temp-buffer
(unless (zerop (cadr (insert-file-contents temp-file)))
(insert ": "))
@@ -181,10 +184,10 @@
(when (buffer-name temp-buffer) (kill-buffer temp-buffer)
;;;###autoload
-(defun clang-format-buffer ( style)
+(defun clang-format-buffer ( style assume-file)
"Use clang-format to format the current buffer according to STYLE."
(interactive)
- (clang-format-region (point-min) (point-max) style))
+ (clang-format-region (point-min) (point-max) style assume-file))
;;;###autoload
(defalias 'clang-format 'clang-format-region)
Index: tools/clang-format/clang-format.el
===
--- tools/clang-format/clang-format.el
+++ tools/clang-format/clang-format.el
@@ -119,7 +119,7 @@
(byte-to-position (1+ byte)
;;;###autoload
-(defun clang-format-region (start end style)
+(defun clang-format-region (start end style assume-file)
"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 active region. If no style is given uses `clang-format-style'."
@@ -131,6 +131,9 @@
(unless style
(setq style clang-format-style))
+ (unless assume-file
+(setq assume-file buffer-file-name))
+
(let ((file-start (clang-format--bufferpos-to-filepos start 'approximate
'utf-8-unix))
(file-end (clang-format--bufferpos-to-filepos end 'approximate
@@ -144,16 +147,16 @@
;; always use ‘utf-8-unix’ and ignore the buffer coding system.
(default-process-coding-system '(utf-8-unix . utf-8-unix)))
(unwind-protect
-(let ((status (call-process-region
- nil nil clang-format-executable
- nil `(,temp-buffer ,temp-file) nil
-
- "-output-replacements-xml"
- "-assume-filename" (or
