Re: [PATCH 1/2] Emacs: Add a new function for balancing bidi control chars
* 2020-08-16 19:28:51+03, Tomi Ollila wrote: > Good stuff -- implementation looks like port of the php code in > >https://www.iamcal.com/understanding-bidirectional-text > > to emacs lisp... anyway nice implementation took be a bit of > time for me to understand it... I don't read PHP and didn't try to read that code at all but the idea is simple enough. > thoughts > > - is it slow to execute it always, pure lisp implementation; > (string-match "[\u202a-\u202e]") could be done before that. > (if it were executed often could loop with `looking-at` >(and then moving point based on match-end) be faster... I don't see any speed issues but if we want to optimize I would create a new sanitize function which walks just once across the characters without using regular expressions. But currently I think it's unnecessary micro optimization. > - *but* adding U+202C's in `notmuch-sanitize` is doing it too early, as > some functions truncate the strings afterwards if those are too long > (e.g. `notmuch-search-insert-authors`) so those get lost.. Good point. This would mean that we shouldn't do "bidi ctrl char balancing" in notmuch-sanitize. We should call the new notmuch-balance-bidi-ctrl-chars function in various places before inserting arbitrary strings to buffer and before combining such strings with other strings. > (what I noticed when looking `notmuch-search-insert-authors` that it uses > `length` to check the length of a string -- but that also counts these bidi > mode changing "characters" (as one char). `string-width` would be better > there -- and probably in many other places.) Yes, definitely string-width when truncating is based on width and when using tabular format in buffers. With that function zero-width characters really have no width. -- /// Teemu Likonen - .-.. http://www.iki.fi/tlikonen/ // OpenPGP: 4E1055DC84E9DFF613D78557719D69D324539450 signature.asc Description: PGP signature ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
[PATCH v2] Emacs: Fix notmuch-message-summary-face definition
Emacs face definition forms are either ((DISPLAY . PLIST) (DISPLAY . PLIST)) or ((DISPLAY PLIST) ;For backward compatibility. (DISPLAY PLIST)) Commit a2388bc56e55da5d5695816818274f8a84b0ed92 (2020-08-08) follows neither of the correct formats. It defines: `class color) (background light)) ,@(and (>= emacs-major-version 27) '(:extend t)) (:background "#f0f0f0")) (((class color) (background dark)) ,@(and (>= emacs-major-version 27) '(:extend t)) (:background "#303030"))) which produces: ((DISPLAY :extend t (:background "#f0f0f0")) (DISPLAY :extend t (:background "#303030"))) And that is wrong format. This change fixes the face definition form to produce: ((DISPLAY :extend t :background "#f0f0f0") (DISPLAY :extend t :background "#303030")) which follows the (DISPLAY . PLIST) format (see above). --- emacs/notmuch.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) * 2020-08-16 15:51:14+02, Jonas Bernoulli wrote: > I would recommend that you > - switch to using the new format > - keep the `:extend' setting on its own line > - keep the `:extend' at the beginning of the list The new format is the only meaningful change but OK. > - use `and' instead of `if' because > - it is better to use `when' instead of `if' when > there is no ELSE part I disagree with that. I think IF is more about return values and WHEN about longer code with side effects. > - it is better to use `and' instead of `when` when > the form is about the returned value, not some > side-effect To me AND is more like multiple condition for "if all the forms are non-nil" and IF is more about return values. Obviously they are techinally the same. Nevertheless, I changed my IF's to AND's so there is now the smallest possible diff in this version. diff --git a/emacs/notmuch.el b/emacs/notmuch.el index babddbb6..04123595 100644 --- a/emacs/notmuch.el +++ b/emacs/notmuch.el @@ -275,10 +275,10 @@ there will be called at other points of notmuch execution." (defface notmuch-message-summary-face `class color) (background light)) ,@(and (>= emacs-major-version 27) '(:extend t)) - (:background "#f0f0f0")) + :background "#f0f0f0") (((class color) (background dark)) ,@(and (>= emacs-major-version 27) '(:extend t)) - (:background "#303030"))) + :background "#303030")) "Face for the single-line message summary in notmuch-show-mode." :group 'notmuch-show :group 'notmuch-faces) -- 2.20.1 ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
[PATCH] test: fix uninitialized variable use in T562-lib-database
Fix a copy paste error of using the boolean ret as a notmuch_status_t,
and uninitialized.
---
test/T562-lib-database.sh | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/test/T562-lib-database.sh b/test/T562-lib-database.sh
index d6097418..dd4f2566 100755
--- a/test/T562-lib-database.sh
+++ b/test/T562-lib-database.sh
@@ -154,11 +154,9 @@ test_expect_equal_file EXPECTED OUTPUT
test_begin_subtest "upgrade a closed db"
cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
{
-notmuch_bool_t ret;
-
EXPECT0(notmuch_database_close (db));
stat = notmuch_database_upgrade (db, NULL, NULL);
-printf ("%d\n", ret == NOTMUCH_STATUS_SUCCESS);
+printf ("%d\n", stat == NOTMUCH_STATUS_SUCCESS);
}
EOF
cat < EXPECTED
--
2.28.0
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [PATCH 1/2] Emacs: Add a new function for balancing bidi control chars
On Sat, Aug 15 2020, Teemu Likonen wrote: > The following Unicode's bidirectional control chars are modal so that > they push a new bidirectional rendering mode to a stack: > > U+202A LEFT-TO-RIGHT EMBEDDING > U+202B RIGHT-TO-LEFT EMBEDDING > U+202D LEFT-TO-RIGHT OVERRIDE > U+202E RIGHT-TO-LEFT OVERRIDE Good stuff -- implementation looks like port of the php code in https://www.iamcal.com/understanding-bidirectional-text to emacs lisp... anyway nice implementation took be a bit of time for me to understand it... thoughts - is it slow to execute it always, pure lisp implementation; (string-match "[\u202a-\u202e]") could be done before that. (if it were executed often could loop with `looking-at` (and then moving point based on match-end) be faster... - *but* adding U+202C's in `notmuch-sanitize` is doing it too early, as some functions truncate the strings afterwards if those are too long (e.g. `notmuch-search-insert-authors`) so those get lost.. - what about https://en.wikipedia.org/wiki/Bidirectional_text#Isolates (was documented more in some page, cannot find it anymore...) (what I noticed when looking `notmuch-search-insert-authors` that it uses `length` to check the length of a string -- but that also counts these bidi mode changing "characters" (as one char). `string-width` would be better there -- and probably in many other places.) (I tried quite a few things, something that could "reset" the stack with e.g. one invisible tab, but no go (or that was filtered as I added it to `notmuch-sanitize` ;), As a final step I did (defun notmuch-sanitize (str) ... - (replace-regexp-in-string "[[:cntrl:]\x7f\u2028\u2029]+" " " str)) + (replace-regexp-in-string + "[\u202A-\u202E\u2066-\u2069]" "" + (replace-regexp-in-string "[[:cntrl:]\x7f\u2028\u2029]+" " " str))) just to test-drop those chars. probably not good enough ;/) Tomi > > Every mode must be terminated with with character U+202C POP > DIRECTIONAL FORMATTING which pops the mode from the stack. The stack > is per paragraph. A new text paragraph resets the rendering mode > changed by these control characters. > > This change adds a new function "notmuch-balance-bidi-ctrl-chars" > which reads its STRING argument and ensures that all push > characters (U+202A, U+202B, U+202D, U+202E) have a pop character > pair (U+202C). The function may add more U+202C characters at the end > of the returned string, or it may remove some U+202C characters. The > returned string is safe in the sense that it won't change the > surrounding bidirectional rendering mode. This function should be used > when sanitizing arbitrary input. > --- > emacs/notmuch-lib.el | 54 > 1 file changed, 54 insertions(+) > > diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el > index 118faf1e..e6252c6c 100644 > --- a/emacs/notmuch-lib.el > +++ b/emacs/notmuch-lib.el > @@ -469,6 +469,60 @@ be displayed." > "[No Subject]" >subject))) > > + > +(defun notmuch-balance-bidi-ctrl-chars (string) > + "Balance bidirectional control chars in STRING. > + > +The following Unicode's bidirectional control chars are modal so > +that they push a new bidirectional rendering mode to a stack: > +U+202A LEFT-TO-RIGHT EMBEDDING, U+202B RIGHT-TO-LEFT EMBEDDING, > +U+202D LEFT-TO-RIGHT OVERRIDE and U+202E RIGHT-TO-LEFT OVERRIDE. > +Every mode must be terminated with with character U+202C POP > +DIRECTIONAL FORMATTING which pops the mode from the stack. The > +stack is per paragraph. A new text paragraph resets the rendering > +mode changed by these control characters. > + > +This function reads the STRING argument and ensures that all push > +characters (U+202A, U+202B, U+202D, U+202E) have a pop character > +pair (U+202C). The function may add more U+202C characters at the > +end of the returned string, or it may remove some U+202C > +characters. The returned string is safe in the sense that it > +won't change the surrounding bidirectional rendering mode. This > +function should be used when sanitizing arbitrary input." > + > + (let ((new-string nil) > + (stack-count 0)) > + > +(cl-flet ((push-char-p (c) > + ;; U+202A LEFT-TO-RIGHT EMBEDDING > + ;; U+202B RIGHT-TO-LEFT EMBEDDING > + ;; U+202D LEFT-TO-RIGHT OVERRIDE > + ;; U+202E RIGHT-TO-LEFT OVERRIDE > + (cl-find c '(?\u202a ?\u202b ?\u202d ?\u202e))) > + (pop-char-p (c) > + ;; U+202C POP DIRECTIONAL FORMATTING > + (eql c ?\u202c))) > + > + (cl-loop for char across string > +do (cond ((push-char-p char) > + (cl-incf stack-count) > + (push char new-string)) > + ((and (pop-char-p char) > + (cl-plusp stack-count)) > + (cl-decf stack-count) > + (push char new-string)) > +
[PATCH] build: clean up sphinx.config
Follow the existing practice and remove it under "distclean", same as sh.config and Makefile.config --- Makefile.local | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.local b/Makefile.local index 156c8ce1..c65cee7c 100644 --- a/Makefile.local +++ b/Makefile.local @@ -294,7 +294,7 @@ CLEAN := $(CLEAN) notmuch notmuch-shared $(notmuch_client_modules) CLEAN := $(CLEAN) version.stamp notmuch-*.tar.gz.tmp CLEAN := $(CLEAN) .deps -DISTCLEAN := $(DISTCLEAN) .first-build-message Makefile.config sh.config +DISTCLEAN := $(DISTCLEAN) .first-build-message Makefile.config sh.config sphinx.config CPPCHECK_STAMPS := $(SRCS:%=.stamps/cppcheck/%) .PHONY: cppcheck -- 2.28.0 ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [PATCH] devel/release-checks.sh: use grep to find copyright year.
On Sun, Aug 16 2020, David Bremner wrote:
> This is quite fragile, but it works for now, unlike the python
> version.
>
> In general it seems conf.py is not intended to be evaluated outside of
> sphinx, as it assumes certain global names (in particular "tags") are
> defined.
> ---
>
> I am going to apply this to unblock the release process, but I
> welcome better solutions.
for this case solution is good enough...
>
> devel/release-checks.sh | 5 +
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/devel/release-checks.sh b/devel/release-checks.sh
> index 7ba94822..cfa208d5 100755
> --- a/devel/release-checks.sh
> +++ b/devel/release-checks.sh
> @@ -178,10 +178,7 @@ esac
> year=`exec date +%Y`
> echo -n "Checking that copyright in documentation contains 2009-$year... "
> # Read the value of variable `copyright' defined in 'doc/conf.py'.
> -# As __file__ is not defined when python command is given from command line,
> -# it is defined before contents of 'doc/conf.py' (which dereferences
> __file__)
> -# is executed.
> -copyrightline=`exec python -c "with open('doc/conf.py') as cf: __file__ =
> ''; exec(cf.read()); print(copyright)"`
> +copyrightline=$(grep ^copyright doc/conf.py)
For consistency and to save one fork(2) $(exec grep ...), but I could
live with this, too ;D
So LGTM.
Tomi
(2) bash forks for subshell and then for grep, even in this trivial case.
Other shells fork only once.
> case $copyrightline in
> *2009-$year*)
> echo Yes. ;;
> --
> 2.28.0
> ___
> notmuch mailing list -- notmuch@notmuchmail.org
> To unsubscribe send an email to notmuch-le...@notmuchmail.org
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org
starting release process for 0.31
I've tagged the first release candidate for 0.31 (0.31_rc0), and uploaded signed tarballs to the usual place [1]. We need to sort out NEWS, and possibly add one or two small patches before final release of 0.31 [1]: https://notmuchmail.org/releases/ signature.asc Description: PGP signature ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
[PATCH] devel/release-checks.sh: use grep to find copyright year.
This is quite fragile, but it works for now, unlike the python
version.
In general it seems conf.py is not intended to be evaluated outside of
sphinx, as it assumes certain global names (in particular "tags") are
defined.
---
I am going to apply this to unblock the release process, but I
welcome better solutions.
devel/release-checks.sh | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/devel/release-checks.sh b/devel/release-checks.sh
index 7ba94822..cfa208d5 100755
--- a/devel/release-checks.sh
+++ b/devel/release-checks.sh
@@ -178,10 +178,7 @@ esac
year=`exec date +%Y`
echo -n "Checking that copyright in documentation contains 2009-$year... "
# Read the value of variable `copyright' defined in 'doc/conf.py'.
-# As __file__ is not defined when python command is given from command line,
-# it is defined before contents of 'doc/conf.py' (which dereferences __file__)
-# is executed.
-copyrightline=`exec python -c "with open('doc/conf.py') as cf: __file__ = '';
exec(cf.read()); print(copyright)"`
+copyrightline=$(grep ^copyright doc/conf.py)
case $copyrightline in
*2009-$year*)
echo Yes. ;;
--
2.28.0
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [PATCH v2 1/3] emacs/tree: introduce notmuch-tree-parent-buffer variable
William Casarin writes: > This variable will be used in a similar fashion to > notmuch-show-parent-buffer. It will be used to navigate between > threads from the parent search buffer. > > Signed-off-by: William Casarin v2 applied to master ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [PATCH] test: update README to reflect dropping upgrade tests
David Bremner writes: > These test databases have been unneeded since ee897cab8. applied to master. d ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [PATCH] Emacs: Fix notmuch-message-summary-face definition
Teemu Likonen writes: > Emacs face definition forms are either > > ((DISPLAY . PLIST) > (DISPLAY . PLIST)) > > or > > ((DISPLAY PLIST) ;For backward compatibility. > (DISPLAY PLIST)) > > Commit a2388bc56e55da5d5695816818274f8a84b0ed92 (2020-08-08) follows > neither of the correct formats. It defines: > > `class color) (background light)) >,@(and (>= emacs-major-version 27) '(:extend t)) >(:background "#f0f0f0")) > (((class color) (background dark)) >,@(and (>= emacs-major-version 27) '(:extend t)) >(:background "#303030"))) > > which produces: > > ((DISPLAY > :extend t (:background "#f0f0f0")) > (DISPLAY > :extend t (:background "#303030"))) > > And that is wrong format. You are right. Sorry about this. It happened because in every other package I patched so far the new format was used and I didn't notice that this wasn't the case here. > This change fixes the face definition form to produce: > > ((DISPLAY > (:background "#f0f0f0" :extend t)) > (DISPLAY > (:background "#303030" :extend t))) > > which follows the (DISPLAY PLIST) format (see above). > --- > emacs/notmuch.el | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/emacs/notmuch.el b/emacs/notmuch.el > index babddbb6..16227b5c 100644 > --- a/emacs/notmuch.el > +++ b/emacs/notmuch.el > @@ -274,11 +274,9 @@ there will be called at other points of notmuch > execution." > > (defface notmuch-message-summary-face >`class color) (background light)) > - ,@(and (>= emacs-major-version 27) '(:extend t)) > - (:background "#f0f0f0")) > + (:background "#f0f0f0" ,@(if (>= emacs-major-version 27) '(:extend t > (((class color) (background dark)) > - ,@(and (>= emacs-major-version 27) '(:extend t)) > - (:background "#303030"))) > + (:background "#303030" ,@(if (>= emacs-major-version 27) '(:extend > t) >"Face for the single-line message summary in notmuch-show-mode." >:group 'notmuch-show >:group 'notmuch-faces) > -- > 2.20.1 I would recommend that you - switch to using the new format - keep the `:extend' setting on its own line - keep the `:extend' at the beginning of the list - use `and' instead of `if' because - it is better to use `when' instead of `if' when there is no ELSE part - it is better to use `and' instead of `when` when the form is about the returned value, not some side-effect Best regards, Jonas ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
[PATCH] Emacs: Fix notmuch-message-summary-face definition
Emacs face definition forms are either ((DISPLAY . PLIST) (DISPLAY . PLIST)) or ((DISPLAY PLIST) ;For backward compatibility. (DISPLAY PLIST)) Commit a2388bc56e55da5d5695816818274f8a84b0ed92 (2020-08-08) follows neither of the correct formats. It defines: `class color) (background light)) ,@(and (>= emacs-major-version 27) '(:extend t)) (:background "#f0f0f0")) (((class color) (background dark)) ,@(and (>= emacs-major-version 27) '(:extend t)) (:background "#303030"))) which produces: ((DISPLAY :extend t (:background "#f0f0f0")) (DISPLAY :extend t (:background "#303030"))) And that is wrong format. This change fixes the face definition form to produce: ((DISPLAY (:background "#f0f0f0" :extend t)) (DISPLAY (:background "#303030" :extend t))) which follows the (DISPLAY PLIST) format (see above). --- emacs/notmuch.el | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/emacs/notmuch.el b/emacs/notmuch.el index babddbb6..16227b5c 100644 --- a/emacs/notmuch.el +++ b/emacs/notmuch.el @@ -274,11 +274,9 @@ there will be called at other points of notmuch execution." (defface notmuch-message-summary-face `class color) (background light)) - ,@(and (>= emacs-major-version 27) '(:extend t)) - (:background "#f0f0f0")) + (:background "#f0f0f0" ,@(if (>= emacs-major-version 27) '(:extend t (((class color) (background dark)) - ,@(and (>= emacs-major-version 27) '(:extend t)) - (:background "#303030"))) + (:background "#303030" ,@(if (>= emacs-major-version 27) '(:extend t) "Face for the single-line message summary in notmuch-show-mode." :group 'notmuch-show :group 'notmuch-faces) -- 2.20.1 ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
