clang-format.el: fix warnings

[augfab]

Summary

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 second 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

[phst]

clang/tools/clang-format/clang-format.el

85

what's the change here?

96

same here, what is being changed?

102

The intention here is that this should be just `replacement' since I'm pretty sure we don't have XML tags like <quote>

115

same here, AIUI this should just be `quote'

[DavidTruby]

I think the other suggestion from the warning should be applied, as I don't think it _is_ intended that "quote" is matched by either of these. It would actually be odd if it did as you'd have two matches.

clang/tools/clang-format/clang-format.el

115

I think you mean "just cursor" here

[augfab]

clang/tools/clang-format/clang-format.el

85

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

96

Same as above, no change, no idea why it's highlighted 😕

102

You are right, thanks. I misunderstood what cl-case does.

115

You are right.

[augfab]

Applied suggestions.

[DavidTruby]

LGTM

[augfab]

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 🙂

[phst]

I'm not super familiar with the process either, but IIRC @sammccall or @djasper have to merge this.

[MyDeveloperDay]

I'm one of the code owners for clang-format, but I'm not an emacs user (don't hate me..), honestly I don't think you'll get a response from anyone others than the clang-format team @owenpan, @HazardyKnusperkeks , @rymiel etc.. but again I can't say if they are emacs users.

If its been accepted by others we can land it.

[MyDeveloperDay]

please mark the comments as done when you've addressed them

[HazardyKnusperkeks]

In D143560#4149733, @phst wrote:

I'm not super familiar with the process either, but IIRC @sammccall or @djasper have to merge this.

Anyone with commit access can do that.

In D143560#4147918, @augfab wrote:

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 🙂

If you don't have commit access yourself, please post your name and email for the commit. Someone will come and commit it on your behalf.

[augfab]

All comments were addressed, marking them as Done.

I don't have commit access, here's my name and email: Augustin Fabre <[e-mail]>.

Thanks!

clang/tools/clang-format/clang-format.el

(let ((nodes (xml-node-children xml-node))
         (incomplete-format (xml-get-attribute xml-node 'incomplete_format))
         replacements
         cursor)
     (dolist (node nodes)
       (when (listp node)
         (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))
                  (error "<replacement> node does not have offset and length attributes"))
                (when (cdr children)
                  (error "More than one child node in <replacement> node"))
 
                (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.
     (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
 is a zero-based file offset, assuming ‘utf-8-unix’ coding."
   (let ((start (clang-format--filepos-to-bufferpos offset 'exact 'utf-8-unix))
         (end (clang-format--filepos-to-bufferpos (+ offset length) 'exact
                                                  'utf-8-unix)))
     (goto-char start)
     (delete-region start end)
     (when text
       (insert text))))
 
 ;; ‘bufferpos-to-filepos’ and ‘filepos-to-bufferpos’ are new in Emacs 25.1.
 ;; Provide fallbacks for older versions.
 (defalias 'clang-format--bufferpos-to-filepos
   (if (fboundp 'bufferpos-to-filepos)