[clang] [emacs][clang-format] Add elisp API for clang-format on git diffs (PR #112792)

via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 20 04:58:33 PST 2024


goldsteinn wrote:


> > 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.
> 
> Fair enough - as noted, this is fairly opinionated - and I don't think your "wrong" for holding the contrary position.
> 
> Keep in mind many people have been using `clang-format.el` for years without this functionality, and will continue to do so. I disagree that this change is simple though, it seems simple on face value - based on the number of replies in the review so far, issues calling out to git & diff with their different versions ... possible optimizations, possible errors when they fail ... is in fact more complicated than may first seem.
> 
> There are some potential bugs that could bite us:
> 
> * How to handle git failing (if git doesn't know about the file... the repository is in some unexpected state... the file could be in the middle of resolving a conflict for e.g.).
> * What if git or diff considers the file to be a binary file.
> * The emacs buffers should use the encoding settings from the buffer that is being edited... do they? (I'd need to double check - at a guess - they don't seem to at the moment).
> 

I don't disagree these are all potential pitfalls (and there are certainly more), I just don't see how having the diff code in a separate project ameliorates any of them. And as stated earlier, I think it in fact complicates them.

https://github.com/llvm/llvm-project/pull/112792


More information about the cfe-commits mailing list