[PATCH] D37903: Fix assume-filename handling in clang-format.el

[Philipp via Phabricator via cfe-commits]

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 <<< 'aaaa = 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