[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