ideasman42 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. --- 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 customized - integrated with other packages that already track changes - such as `diff-hl` could be used. - All the details of integrating diff-hl/non-git version control, platform support (proper error messages if `diff` 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. The main down side is users who rely on this behavior need to install an additional package. https://github.com/llvm/llvm-project/pull/112792 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
