Mark Walters <markwalters1009@gmail.com> writes:

[...]

> Version 1 of this patch (with some discussion) is at
> id:1477191835-17828-1-git-send-email-markwalters1009@gmail.com
>
> The general consensus is that we should not define functions outside
> our namespace, even when they are just backports of functions from
> later emacs.
>
> rlb on irc gave one additional reason not mentioned earlier in the
> thread -- it could be that some other package choses to test for the
> setq-local say and perhaps draws incorrect conclusions about the
> environment. In particular this would be a case where we were breaking
> otherwise working packages.

Ahh, good point.

Thanks for sticking with the patch.


> I think it makes sense to included the whole of read-char-choice, not
> some cutdown version, as we may use other functionality from it later
> (eg help-forms etc), and it would be confusing if the change only
> worked on emacs 24.
>
> Finally, I have two questions
>
> 1) please could someone with emacs 23 see if the testsuite passes? My
> system with emacs 23 is so outdated the test suite doesn't run (wrong
> python versions I think).

I tried.

FWIW, the patch didn't apply cleanly for me, though I'm not used to
slurping patches around in email.  I used: "git apply -3 mark.patch".
"git apply --reject mark.patch" left a few .rej files that were easily
applied by hand (it looked like some code had moved too far?).

I'm stuck on an Ubuntu trusty release, which is rather old and has a
packaged emacs 23.  I'm not sure how useful my results are, though,
because I show a lot of test errors regardless.  This may be a result of
me compiling my own xapian, which the notmuch build system does not
handle particularly gracefully.  Most failures are of the form "A Xapian
exception occurred performing query", and unrelated to your patch.

Before your patch I got the expected byte compiler warnings:

% XAPIAN_CONFIG=$HOME/opt/xapian-core-1.4.0/bin/xapian-config ./configure
% LD_RUN_PATH=$HOME/opt/xapian-core-1.4.0/lib make test

[...]

notmuch-company.el:95:1:Warning: the function `setq-local' is not known to be
    defined.
notmuch-address.el:351:1:Warning: the function `setq-local' is not known to be
    defined.
notmuch-maildir-fcc.el:376:1:Warning: the function `read-char-choice' is not
    known to be defined.

[...then I see...]

Notmuch test suite complete.
635/783 tests passed.
2 broken tests failed as expected.
43 tests failed.
103 tests skipped.

[...with this patch the byte compiler shuts up, but I get the same test
suite errors...]

Notmuch test suite complete.
635/783 tests passed.
2 broken tests failed as expected.
43 tests failed.
103 tests skipped.

[...but not to worry, I get the same running Emacs 25 too...]

I'll attach full output of the notmuch test run.


> 2) Is the copyright notice I have included above the two functions
> sufficient, and suitably placed?

An alternative is to place these functions in a new file named something
like notmuch-backport.el, with clear FSF copyright.  Since what
constitutes "functions in this section" may drift over time, the rigor
of a separate file might be desirable.


> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -966,6 +966,78 @@ status."
>  (defvar notmuch-show-process-crypto nil)
>  (make-variable-buffer-local 'notmuch-show-process-crypto)
>  
> +;; Compatibility functions for emacs 23.
> +
> +;; The functions in this section are copied from eamcs 24.4 and are

typo: eamcs