On Sun, May 26 2019, David Bremner wrote:

> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> On Sun 2019-05-26 09:01:46 -0300, David Bremner wrote:
>>> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>>>
>>>> This is the third revision of the series originally posted at
>>>> id:20190424183113.29242-1-dkg@fifthhorseman.net (revision 2 was at
>>>> id:20190520032228.27420-1-dkg@fifthhorseman.net)
>>>>
>>>> This series addresses comments raised by David Bremner in his review.
>>>> Thanks, Bremner!
>>>
>>> I pushed this version, with 4 minor whitespace fixups (inserted spaces
>>> and/or deleted blank lines) that I missed in my previous review.
>>
>> Thanks for these fixes, Bremner.  If you have a specific set of tooling
>> that you use that i can adopt to catch those kinds of coding convention
>> mishaps before submitting, i'd be happy to adopt it so things are
>> "clean".  Bonus points if it integrates into emacs via .dir-locals.el or
>> something :P
>
> To be honest my main mechanism for catching those problems is Tomi
> ;). 

I used to have simple trailing whitespace cleanup hook 
(with its problems). Since Now 2012 I've used ethan-wspace.el which
does cleanups only initially clean content (and higlights if not --
my favorite feature). Like w/ notmuch I also use a bit different version:

localhost$ diff -u vc/ext/ethan-wspace/lisp/ethan-wspace.el dotdir/elisp/ethan-wspace.el
--- vc/ext/ethan-wspace/lisp/ethan-wspace.el     2019-05-27 23:32:26.625456294 +0300
+++ dotdir/elisp/ethan-wspace.el      2019-05-27 23:38:34.150404797 +0300
@@ -800,8 +800,7 @@
           (setq require-final-newline nil))
 
         (when indent-tabs-mode
-          (setq ethan-wspace-errors
-                (remove 'tabs ethan-wspace-errors)))
+                  (ethan-wspace-highlight-tabs-mode))
 
         (run-hooks 'ethan-wspace-errors-in-buffer-hook)
         (ethan-wspace-update-buffer)
zsh: exit 1     diff -u ~/vc/ext/ethan-wspace/lisp/ethan-wspace.el 

(applied latest change from github just now, changed there 2019-05-21)

> There is also a reasonable uncrustify configuration that I don't use
> as often as I should, mainly because the baseline in various files is
> not there. Perhaps if we did more some global whitespace cleanup this
> would be more helpful. To try it out run
>
> % uncrustify -c devel/uncrustify.cfg --replace $files
>
> then
>
> % git diff
>
> to see what changed.
>
> % find -name .git -prune -o -type f -a -regex '.*[.]\(c\|h\|cc\)' -print -exec uncrustify -c devel/uncrustify.cfg --replace {} \;
>
> yields a whopping 1933 insertions and 1903 deletions. Perhaps there are
> some places where the uncrustify config needs tuning, but none of the
> output looked crazy to me. Two things I noticed causing lots of changes
>
> 1) "!foo" -> "! foo"
> 2) putting a leading * in front of multi-line comments

Last time I played w/ uncrustify it was like 2014 -- and I recall there
were things that uncrustify just could not do (more than above) -- like
indenting differently than emacs would do (and IMO something ugly way).

So I wonder if we ever can use uncrustify as tool to verify change
formatting (i.e. identical output or FAIL)...

... perhaps newer uncrustify (latest 0.69, in 2014 version was 0.60)
(with modified uncrustify.cfg) could help there (I also have done
some experiments with "uncrustify postprocessor"; I should take some
time to see how that works with latest tool...)

> If we do decide to rip off the bandage, that will cause a certain amount
> of rebasing pain for any patch series in flight; now (i.e after the
> release) is actually a pretty good time from my point of view, but
> others (e.g. you) might feel differently.

If uncrustifying can be done (even partially), IMO it could be done;
patch series authors just have to revisit their changes (keeping those
live is their duty anyway).


Tomi
_______________________________________________
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch