[PATCH] D143560: clang-format.el: fix warnings
augfab marked 5 inline comments as done. augfab added a comment. All comments were addressed, marking them as Done. I don't have commit access, here's my name and email: Augustin Fabre . Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143560/new/ https://reviews.llvm.org/D143560 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D143560: clang-format.el: fix warnings
augfab created this revision.
augfab added reviewers: sammccall, phst, djasper.
augfab added a project: clang-format.
Herald added a project: All.
augfab requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Emacs trunk complained about:
Warning (comp): clang-format.el:87:15: Warning: Case 'replacement will match
‘quote’. If that’s intended, write (replacement quote) instead. Otherwise,
don’t quote ‘replacement’.
Warning (comp): clang-format.el:98:15: Warning: Case 'cursor will match
‘quote’. If that’s intended, write (cursor quote) instead. Otherwise, don’t
quote ‘cursor’.
Fixed the syntax by applying the suggestion.
Checked that the code works like before the change by stepping through the
execution of the function.
Ran tests with Emacs 26.3 and Emacs trunk with:
$ emacs -Q -batch -l clang/tools/clang-format/clang-format.el -l
clang/tools/clang-format/clang-format-test.el -f ert-run-tests-batch-and-exit
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D143560
Files:
clang/tools/clang-format/clang-format.el
Index: clang/tools/clang-format/clang-format.el
===
--- clang/tools/clang-format/clang-format.el
+++ clang/tools/clang-format/clang-format.el
@@ -82,29 +82,29 @@
(let* ((children (xml-node-children node))
(text (car children)))
(cl-case (xml-node-name node)
-('replacement
+((replacement quote)
(let* ((offset (xml-get-attribute-or-nil node 'offset))
(length (xml-get-attribute-or-nil node 'length)))
(when (or (null offset) (null length))
(error " node does not have offset and length
attributes"))
(when (cdr children)
(error "More than one child node in node"))
(setq offset (string-to-number offset))
(setq length (string-to-number length))
(push (list offset length text) replacements)))
-('cursor
+((cursor quote)
(setq cursor (string-to-number text)))
;; Sort by decreasing offset, length.
(setq replacements (sort (delq nil replacements)
(lambda (a b)
(or (> (car a) (car b))
(and (= (car a) (car b))
(> (cadr a) (cadr b)))
(list replacements cursor (string= incomplete-format "true"
(defun clang-format--replace (offset length &optional text)
"Replace the region defined by OFFSET and LENGTH with TEXT.
OFFSET and LENGTH are measured in bytes, not characters. OFFSET
Index: clang/tools/clang-format/clang-format.el
===
--- clang/tools/clang-format/clang-format.el
+++ clang/tools/clang-format/clang-format.el
@@ -82,29 +82,29 @@
(let* ((children (xml-node-children node))
(text (car children)))
(cl-case (xml-node-name node)
-('replacement
+((replacement quote)
(let* ((offset (xml-get-attribute-or-nil node 'offset))
(length (xml-get-attribute-or-nil node 'length)))
(when (or (null offset) (null length))
(error " node does not have offset and length attributes"))
(when (cdr children)
(error "More than one child node in node"))
(setq offset (string-to-number offset))
(setq length (string-to-number length))
(push (list offset length text) replacements)))
-('cursor
+((cursor quote)
(setq cursor (string-to-number text)))
;; Sort by decreasing offset, length.
(setq replacements (sort (delq nil replacements)
(lambda (a b)
(or (> (car a) (car b))
(and (= (car a) (car b))
(> (cadr a) (cadr b)))
(list replacements cursor (string= incomplete-format "true"
(defun clang-format--replace (offset length &optional text)
"Replace the region defined by OFFSET and LENGTH with TEXT.
OFFSET and LENGTH are measured in bytes, not characters. OFFSET
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D143560: clang-format.el: fix warnings
augfab added inline comments.
Comment at: clang/tools/clang-format/clang-format.el:70
(cl-case (xml-node-name node)
-('replacement
+((replacement quote)
(let* ((offset (xml-get-attribute-or-nil node 'offset))
phst wrote:
> what's the change here?
There is no change here, nor on line 80.
I don't understand why Phabricator highlights these two lines in red/green.
The raw diff doesn't show these two lines:
https://reviews.llvm.org/file/data/u6j4564axwglt6pu42ff/PHID-FILE-fa54axif3rocku7zfmix/D143560.diff
Comment at: clang/tools/clang-format/clang-format.el:80
(push (list offset length text) replacements)))
-('cursor
+((cursor quote)
(setq cursor (string-to-number text)))
phst wrote:
> same here, what is being changed?
Same as above, no change, no idea why it's highlighted 😕
Comment at: clang/tools/clang-format/clang-format.el:85
(lambda (a b)
(or (> (car a) (car b))
(and (= (car a) (car b))
phst wrote:
> The intention here is that this should be just `replacement' since I'm pretty
> sure we don't have XML tags like
You are right, thanks. I misunderstood what `cl-case` does.
Comment at: clang/tools/clang-format/clang-format.el:96
'utf-8-unix)))
(goto-char start)
(delete-region start end)
DavidTruby wrote:
> phst wrote:
> > same here, AIUI this should just be `quote'
> I think you mean "just cursor" here
You are right.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143560/new/
https://reviews.llvm.org/D143560
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D143560: clang-format.el: fix warnings
augfab updated this revision to Diff 495815.
augfab edited the summary of this revision.
augfab added a comment.
Applied suggestions.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143560/new/
https://reviews.llvm.org/D143560
Files:
clang/tools/clang-format/clang-format.el
Index: clang/tools/clang-format/clang-format.el
===
--- clang/tools/clang-format/clang-format.el
+++ clang/tools/clang-format/clang-format.el
@@ -82,7 +82,7 @@
(let* ((children (xml-node-children node))
(text (car children)))
(cl-case (xml-node-name node)
-('replacement
+(replacement
(let* ((offset (xml-get-attribute-or-nil node 'offset))
(length (xml-get-attribute-or-nil node 'length)))
(when (or (null offset) (null length))
@@ -93,7 +93,7 @@
(setq offset (string-to-number offset))
(setq length (string-to-number length))
(push (list offset length text) replacements)))
-('cursor
+(cursor
(setq cursor (string-to-number text)))
;; Sort by decreasing offset, length.
Index: clang/tools/clang-format/clang-format.el
===
--- clang/tools/clang-format/clang-format.el
+++ clang/tools/clang-format/clang-format.el
@@ -82,7 +82,7 @@
(let* ((children (xml-node-children node))
(text (car children)))
(cl-case (xml-node-name node)
-('replacement
+(replacement
(let* ((offset (xml-get-attribute-or-nil node 'offset))
(length (xml-get-attribute-or-nil node 'length)))
(when (or (null offset) (null length))
@@ -93,7 +93,7 @@
(setq offset (string-to-number offset))
(setq length (string-to-number length))
(push (list offset length text) replacements)))
-('cursor
+(cursor
(setq cursor (string-to-number text)))
;; Sort by decreasing offset, length.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D143560: clang-format.el: fix warnings
augfab added a comment. Hi @phst , is anything left for me to do before this patch is merged? This is my first patch to LLVM so I'm not too familiar with the process. Thanks for your help 🙂 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143560/new/ https://reviews.llvm.org/D143560 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
