Re: [PATCH] emacs: split-window-sensibly in tree mode with open message

2020-05-09 Thread Radu Butoi
Hi David,

David Bremner  writes:

> We seem to have got into the habit of updating the NEWS file right
> before release. This seems to work OK, and avoids some conflicts between
> patch series.

Makes sense, probably something like:

When opening message pane in tree-mode, split window horizontally or vertically 
depending on available space, set by split-{width,height}-threshold.

Otherwise, does the patch look good? split-window-sensibly seems to be a 
drop-in replacement for split-window-*.

Best,
Radu
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 1/2 v2] test-lib: mark function variables as local

2020-05-09 Thread Tomi Ollila
On Fri, May 08 2020, Daniel Kahn Gillmor wrote:

> Several functions in test/test-lib.sh used variable names that are
> also used outside of those functions (e.g. $output and $expected are
> used in many of the test scripts), but they are not expected to
> communicate via those variables.
>
> We mark those variables "local" within test-lib.sh so that they do not
> get clobbered when used outside test-lib.
>
> We also move the local variable declarations to beginning of each
> function, to avoid weird gotchas with local variable declarations as
> described in https://tldp.org/LDP/abs/html/localvar.html.

LGTM. I have to tolerate 'we' (passive) as I seem to use those myself, too
(although communicating a bit different thing...) =D

Tomi

> Signed-off-by: Daniel Kahn Gillmor 
> ---
>  test/test-lib.sh | 31 ++-
>  1 file changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 5c8eab7c..58972339 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -109,7 +109,6 @@ unset ALTERNATE_EDITOR
>  
>  add_gnupg_home ()
>  {
> -local output
>  [ -e "${GNUPGHOME}/gpg.conf" ] && return
>  _gnupg_exit () { gpgconf --kill all 2>/dev/null || true; }
>  at_exit_function _gnupg_exit
> @@ -345,13 +344,14 @@ trap 'trap_signal' HUP INT TERM
>  # to the message and encrypting/signing.
>  emacs_deliver_message ()
>  {
> -local subject="$1"
> -local body="$2"
> +local subject body smtp_dummy_pid smtp_dummy_port
> +subject="$1"
> +body="$2"
>  shift 2
>  # before we can send a message, we have to prepare the FCC maildir
>  mkdir -p "$MAIL_DIR"/sent/{cur,new,tmp}
>  # eval'ing smtp-dummy --background will set smtp_dummy_pid and -_port
> -local smtp_dummy_pid= smtp_dummy_port=
> +smtp_dummy_pid= smtp_dummy_port=
>  eval `$TEST_DIRECTORY/smtp-dummy --background sent_message`
>  test -n "$smtp_dummy_pid" || return 1
>  test -n "$smtp_dummy_port" || return 1
> @@ -391,13 +391,14 @@ emacs_deliver_message ()
>  # new" after message delivery
>  emacs_fcc_message ()
>  {
> -local nmn_args=''
> +local nmn_args subject body
> +nmn_args=''
>  while [[ "$1" =~ ^-- ]]; do
>  nmn_args="$nmn_args $1"
>  shift
>  done
> -local subject="$1"
> -local body="$2"
> +subject="$1"
> +body="$2"
>  shift 2
>  # before we can send a message, we have to prepare the FCC maildir
>  mkdir -p "$MAIL_DIR"/sent/{cur,new,tmp}
> @@ -427,6 +428,7 @@ emacs_fcc_message ()
>  # number of messages.
>  add_email_corpus ()
>  {
> +local corpus
>  corpus=${1:-default}
>  
>  rm -rf ${MAIL_DIR}
> @@ -457,6 +459,7 @@ test_begin_subtest ()
>  # name.
>  test_expect_equal ()
>  {
> + local output expected testname
>   exec 1>&6 2>&7  # Restore stdout and stderr
>   if [ -z "$inside_subtest" ]; then
>   error "bug in the test script: test_expect_equal without 
> test_begin_subtest"
> @@ -483,6 +486,7 @@ test_expect_equal ()
>  # Like test_expect_equal, but takes two filenames.
>  test_expect_equal_file ()
>  {
> + local file1 file2 testname basename1 basename2
>   exec 1>&6 2>&7  # Restore stdout and stderr
>   if [ -z "$inside_subtest" ]; then
>   error "bug in the test script: test_expect_equal_file without 
> test_begin_subtest"
> @@ -512,10 +516,11 @@ test_expect_equal_file ()
>  # canonicalized before diff'ing.  If an argument cannot be parsed, it
>  # is used unchanged so that there's something to diff against.
>  test_expect_equal_json () {
> +local script output expected
>  # The test suite forces LC_ALL=C, but this causes Python 3 to
>  # decode stdin as ASCII.  We need to read JSON in UTF-8, so
>  # override Python's stdio encoding defaults.
> -local script='import json, sys; json.dump(json.load(sys.stdin), 
> sys.stdout, sort_keys=True, indent=4)'
> +script='import json, sys; json.dump(json.load(sys.stdin), sys.stdout, 
> sort_keys=True, indent=4)'
>  output=$(echo "$1" | PYTHONIOENCODING=utf-8 $NOTMUCH_PYTHON -c "$script" 
> \
>  || echo "$1")
>  expected=$(echo "$2" | PYTHONIOENCODING=utf-8 $NOTMUCH_PYTHON -c 
> "$script" \
> @@ -540,6 +545,7 @@ test_sort_json () {
>  # read the source of test/json_check_nodes.py (or the output when
>  # invoking it without arguments) for an explanation of the syntax.
>  test_json_nodes () {
> +local output
>  exec 1>&6 2>&7   # Restore stdout and stderr
>   if [ -z "$inside_subtest" ]; then
>   error "bug in the test script: test_json_eval without 
> test_begin_subtest"
> @@ -561,6 +567,7 @@ test_json_nodes () {
>  }
>  
>  test_emacs_expect_t () {
> + local result
>   test "$#" = 1 ||
>   error "bug in the test script: not 1 parameter to test_emacs_expect_t"
>   if [ -z "$inside_subtest" ]; then
> @@ -653,7 +660,8 @@ notmuch_json_show_sanitize 

Re: [PATCH 1/2 v2] test-lib: mark function variables as local

2020-05-09 Thread David Bremner
Daniel Kahn Gillmor  writes:

> Several functions in test/test-lib.sh used variable names that are
> also used outside of those functions (e.g. $output and $expected are
> used in many of the test scripts), but they are not expected to
> communicate via those variables.
>
> We mark those variables "local" within test-lib.sh so that they do not
> get clobbered when used outside test-lib.
>
> We also move the local variable declarations to beginning of each
> function, to avoid weird gotchas with local variable declarations as
> described in https://tldp.org/LDP/abs/html/localvar.html.

Pushed this one to master.

I'm confused about where to apply 2/2. If I apply it on top of (updated)
master, it causes test failures. If I apply after the rest of the
patches in this thread then presumably there is some interval where the
build is broken (if only for certain GMime versions).

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch