[PATCH] devel: two scripts for checking proposed changes

[PATCH] devel: two scripts for checking proposed changes

[David Bremner]

It took me a bit of effort to get the correct oneliner to reindent
elisp from the command line, so I saved the results as
'reindent-elisp'.

'check-notmuch-commit' is an updated version of a script I have been
using (although not always as consistently as I should) before sending
patches to the list.

Although it requires a bit more tooling, encouraging people to use
check-notmuch-commit might reduce the number of round trips to the
list for style nitpicks.
---
 devel/check-notmuch-commit | 20 ++++++++++++++++++++
 devel/reindent-elisp       |  8 ++++++++
 2 files changed, 28 insertions(+)
 create mode 100755 devel/check-notmuch-commit
 create mode 100755 devel/reindent-elisp

diff --git a/devel/check-notmuch-commit b/devel/check-notmuch-commit
new file mode 100755
index 00000000..98a19a64
--- /dev/null
+++ b/devel/check-notmuch-commit
@@ -0,0 +1,20 @@
+#!/bin/sh
+
+# Usage suggestion:
+#   git rebase -i --exec devel/check-notmuch-commit origin/master
+
+set -e
+make test
+for file in $(git diff --name-only HEAD^); do
+    case $file in
+	*.c|*.h|*.cc|*.hh)
+            uncrustify --replace -c $(dirname "$0")/uncrustify.cfg "$file"
+	    ;;
+	*.el)
+            $(dirname "$0")/reindent-elisp "$file"
+	    ;;
+    esac
+done
+
+git diff --quiet
+
diff --git a/devel/reindent-elisp b/devel/reindent-elisp
new file mode 100755
index 00000000..f6ce3844
--- /dev/null
+++ b/devel/reindent-elisp
@@ -0,0 +1,8 @@
+#!/bin/sh
+
+if [ $# -lt 1 ]; then
+    printf "usage: $0 <path.el>\n"
+    exit 1
+fi
+
+emacs -Q --batch $1 --eval '(indent-region (point-min) (point-max) nil)' -f save-buffer
-- 
2.30.2

Re: [PATCH] devel: two scripts for checking proposed changes

[Tomi Ollila]

On Mon, Jun 28 2021, David Bremner wrote:

> It took me a bit of effort to get the correct oneliner to reindent
> elisp from the command line, so I saved the results as
> 'reindent-elisp'.
>
> 'check-notmuch-commit' is an updated version of a script I have been
> using (although not always as consistently as I should) before sending
> patches to the list.
>
> Although it requires a bit more tooling, encouraging people to use
> check-notmuch-commit might reduce the number of round trips to the
> list for style nitpicks.
> ---
>  devel/check-notmuch-commit | 20 ++++++++++++++++++++
>  devel/reindent-elisp       |  8 ++++++++
>  2 files changed, 28 insertions(+)
>  create mode 100755 devel/check-notmuch-commit
>  create mode 100755 devel/reindent-elisp
>
> diff --git a/devel/check-notmuch-commit b/devel/check-notmuch-commit
> new file mode 100755
> index 00000000..98a19a64
> --- /dev/null
> +++ b/devel/check-notmuch-commit
> @@ -0,0 +1,20 @@
> +#!/bin/sh
> +
> +# Usage suggestion:
> +#   git rebase -i --exec devel/check-notmuch-commit origin/master
> +
> +set -e
> +make test

This may fail miserably (or is painfully slow (doing configure and make...)

> +for file in $(git diff --name-only HEAD^); do

is this also mentioning deleted files... ? (--diff-filter=AM) ?

> +    case $file in
> +	*.c|*.h|*.cc|*.hh)
> +            uncrustify --replace -c $(dirname "$0")/uncrustify.cfg "$file"

dirname "$0" could be resolved once before loop.

> +	    ;;
> +	*.el)
> +            $(dirname "$0")/reindent-elisp "$file"
> +	    ;;
> +    esac
> +done
> +
> +git diff --quiet
> +
> diff --git a/devel/reindent-elisp b/devel/reindent-elisp
> new file mode 100755
> index 00000000..f6ce3844
> --- /dev/null
> +++ b/devel/reindent-elisp
> @@ -0,0 +1,8 @@
> +#!/bin/sh
> +
> +if [ $# -lt 1 ]; then

if [ $# -ne 1 ] ... (see at the end)

> +    printf "usage: $0 <path.el>\n"

Angle brackets are bad in example -- if copy-pasted to terminal (and badly
edited) does redirections...

> +    exit 1
> +fi
> +
> +emacs -Q --batch $1 --eval '(indent-region (point-min) (point-max) nil)'
> -f save-buffer

... as here is $1 -- which should be quoted as "$1"

> -- 
> 2.30.2

Re: [PATCH] devel: two scripts for checking proposed changes

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

>> +
>> +set -e
>> +make test
>
> This may fail miserably (or is painfully slow (doing configure and make...)
>

At least for my use case, running the tests for each commit is the main
point. The formatting stuff is nice to have, but only if the tests run.
I could invoke the notmuch-test script directly, but that is
arguably slightly less robust because of the need to create test-binaries.

>> +for file in $(git diff --name-only HEAD^); do
>
> is this also mentioning deleted files... ? (--diff-filter=AM) ?

Oh, good point.

>
>> +    case $file in
>> +	*.c|*.h|*.cc|*.hh)
>> +            uncrustify --replace -c $(dirname "$0")/uncrustify.cfg "$file"
>
> dirname "$0" could be resolved once before loop.
>

yes, although I'm not sure it's a win?

>> +	    ;;
>> +	*.el)
>> +            $(dirname "$0")/reindent-elisp "$file"
>> +	    ;;
>> +    esac
>> +done
>> +
>> +git diff --quiet
>> +
>> diff --git a/devel/reindent-elisp b/devel/reindent-elisp
>> new file mode 100755
>> index 00000000..f6ce3844
>> --- /dev/null
>> +++ b/devel/reindent-elisp
>> @@ -0,0 +1,8 @@
>> +#!/bin/sh
>> +
>> +if [ $# -lt 1 ]; then
>
> if [ $# -ne 1 ] ... (see at the end)
>
OK
>> +    printf "usage: $0 <path.el>\n"
>
> Angle brackets are bad in example -- if copy-pasted to terminal (and badly
> edited) does redirections...

ack

>
>> +    exit 1
>> +fi
>> +
>> +emacs -Q --batch $1 --eval '(indent-region (point-min) (point-max) nil)'
>> -f save-buffer
>
> ... as here is $1 -- which should be quoted as "$1"

OK

>
>> -- 
>> 2.30.2

Re: [PATCH] devel: two scripts for checking proposed changes

[Tomi Ollila]

On Tue, Jun 29 2021, David Bremner wrote:

> Tomi Ollila <tomi.ollila@iki.fi> writes:
>
>>> +
>>> +set -e
>>> +make test
>>
>> This may fail miserably (or is painfully slow (doing configure and make...)
>>
>
> At least for my use case, running the tests for each commit is the main
> point. The formatting stuff is nice to have, but only if the tests run.
> I could invoke the notmuch-test script directly, but that is
> arguably slightly less robust because of the need to create test-binaries.
>
>>> +for file in $(git diff --name-only HEAD^); do
>>
>> is this also mentioning deleted files... ? (--diff-filter=AM) ?
>
> Oh, good point.
>
>>
>>> +    case $file in
>>> +	*.c|*.h|*.cc|*.hh)
>>> +            uncrustify --replace -c $(dirname "$0")/uncrustify.cfg "$file"
>>
>> dirname "$0" could be resolved once before loop.
>>
>
> yes, although I'm not sure it's a win?

actually (IMO) the 'winnest' option is to:

first have

    unset dn0

and then in all cases of $(dirname "$0") to be replaced with

    "${dn0=$(dirname "$0")}"

In this case $(dirname "$0") is executed at most once -- but zero times
in case the result is never used. (note also quotes around the whole
expression :)

Tomi

>
>>> +	    ;;
>>> +	*.el)
>>> +            $(dirname "$0")/reindent-elisp "$file"
>>> +	    ;;
>>> +    esac
>>> +done
>>> +
>>> +git diff --quiet
>>> +
>>> diff --git a/devel/reindent-elisp b/devel/reindent-elisp
>>> new file mode 100755
>>> index 00000000..f6ce3844
>>> --- /dev/null
>>> +++ b/devel/reindent-elisp
>>> @@ -0,0 +1,8 @@
>>> +#!/bin/sh
>>> +
>>> +if [ $# -lt 1 ]; then
>>
>> if [ $# -ne 1 ] ... (see at the end)
>>
> OK
>>> +    printf "usage: $0 <path.el>\n"
>>
>> Angle brackets are bad in example -- if copy-pasted to terminal (and badly
>> edited) does redirections...
>
> ack
>
>>
>>> +    exit 1
>>> +fi
>>> +
>>> +emacs -Q --batch $1 --eval '(indent-region (point-min) (point-max) nil)'
>>> -f save-buffer
>>
>> ... as here is $1 -- which should be quoted as "$1"
>
> OK
>
>>
>>> -- 
>>> 2.30.2
> _______________________________________________
> notmuch mailing list -- notmuch@notmuchmail.org
> To unsubscribe send an email to notmuch-leave@notmuchmail.org

[PATCH v2] devel: script for checking a commit (series)

[David Bremner]

'check-notmuch-commit' is an updated version of a script I have been
using (although not always as consistently as I should) before sending
patches to the list.

Although it requires a bit more tooling, encouraging people to use
check-notmuch-commit might reduce the number of round trips to the
list for style nitpicks.
---
 devel/check-notmuch-commit | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100755 devel/check-notmuch-commit

Changes since last round:

~ '--quick' argument to avoid expensive build/test
~ use Tomi's trick to avoid multiple calls to dirname
~ merge reindent-lisp into the main script. This eases development
  (only one script to put in path) and reduces execs.
~ include quoting fix for call to emacs  
~ add diff-filter option

diff --git a/devel/check-notmuch-commit b/devel/check-notmuch-commit
new file mode 100755
index 00000000..99ed229d
--- /dev/null
+++ b/devel/check-notmuch-commit
@@ -0,0 +1,32 @@
+#!/bin/sh
+
+# Usage suggestion:
+#   git rebase -i --exec devel/check-notmuch-commit origin/master
+
+set -e
+
+quick=0
+case "$1" in
+    -q|-Q|--quick)
+        quick=1
+        ;;
+esac
+
+if [ $quick = 0 ]; then
+    make test
+fi
+
+unset uconf
+for file in $(git diff --name-only --diff-filter=AM HEAD^); do
+    case $file in
+	*.c|*.h|*.cc|*.hh)
+            uncrustify --replace -c "${uconf=$(dirname "$0")/uncrustify.cfg}" "$file"
+	    ;;
+	*.el)
+	    emacs -Q --batch "$file" --eval '(indent-region (point-min) (point-max) nil)' -f save-buffer
+	    ;;
+    esac
+done
+
+git diff --quiet
+
-- 
2.33.0

Re: [PATCH v2] devel: script for checking a commit (series)

[Tomi Ollila]

On Sun, Oct 10 2021, David Bremner wrote:

> 'check-notmuch-commit' is an updated version of a script I have been
> using (although not always as consistently as I should) before sending
> patches to the list.
>
> Although it requires a bit more tooling, encouraging people to use
> check-notmuch-commit might reduce the number of round trips to the
> list for style nitpicks.
> ---
>  devel/check-notmuch-commit | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>  create mode 100755 devel/check-notmuch-commit
>
> Changes since last round:
>
> ~ '--quick' argument to avoid expensive build/test
> ~ use Tomi's trick to avoid multiple calls to dirname
> ~ merge reindent-lisp into the main script. This eases development
>   (only one script to put in path) and reduces execs.
> ~ include quoting fix for call to emacs  
> ~ add diff-filter option
>
> diff --git a/devel/check-notmuch-commit b/devel/check-notmuch-commit
> new file mode 100755
> index 00000000..99ed229d
> --- /dev/null
> +++ b/devel/check-notmuch-commit
> @@ -0,0 +1,32 @@
> +#!/bin/sh
> +
> +# Usage suggestion:
> +#   git rebase -i --exec devel/check-notmuch-commit origin/master
> +
> +set -e
> +
> +quick=0
> +case "$1" in
> +    -q|-Q|--quick)
> +        quick=1
> +        ;;
> +esac
> +
> +if [ $quick = 0 ]; then
> +    make test
> +fi
> +
> +unset uconf
> +for file in $(git diff --name-only --diff-filter=AM HEAD^); do
> +    case $file in
> +	*.c|*.h|*.cc|*.hh)
> +            uncrustify --replace -c "${uconf=$(dirname "$0")/uncrustify.cfg}" "$file"

The above line does not have TAB, otherwise looks good to me.

(there might be other tab/spaces inconsistencies, but this was local enough for me
to notice ;/)

Tomi

> +	    ;;
> +	*.el)
> +	    emacs -Q --batch "$file" --eval '(indent-region (point-min) (point-max) nil)' -f save-buffer
> +	    ;;
> +    esac
> +done
> +
> +git diff --quiet
> +
> -- 
> 2.33.0

Re: [PATCH v2] devel: script for checking a commit (series)

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> On Sun, Oct 10 2021, David Bremner wrote:
>

> The above line does not have TAB, otherwise looks good to me.
>
> (there might be other tab/spaces inconsistencies, but this was local enough for me
> to notice ;/)

applied, after (blush) whitespace cleanup.

d