Ruijie Yu via "General discussions about Org-mode." <emacs-orgmode@gnu.org> writes:
> Ihor Radchenko <yanta...@posteo.net> writes: > >> (let ((org-time-stamp-formats '("%Y-%m-%d" . "%Y-%m-%d %H:%M"))) >> (org-element-timestamp-interpreter timestamp nil)) > > Thanks for the pointer. I have made this into a macro and redid my > changes accordingly. Please also let me know if more work should be > dedicated to avoid code duplication in tests (I see a lot of duplication > in the portions I touched in testing/lisp/test-org.el). > > What currently still fails unexpectedly: > > zh_CN.UTF-8: > test-org-clock/clocktable/lang > - This test only fails in batch mode, as I mentioned up-thread. I > might try to convert this into a test that expects failure > depending on whether Emacs is in batch mode. > test-org-colview/columns-width > - This patch did not attempt to fix it. I posted a diff file under > v2 (not included in this patch), but that didn't work either. > > ja_JA.UTF-8: > test-org-colview/columns-width > - See above. > > fr_FR.UTF-8: > test-org-clok/org-clock-timestamps-change > - This is tracked in the other thread I created, and won't be fixed > by this patch. > > en_US.UTF-8 and de_DE.UTF-8: > No unexpected test failures. > > [2. text/x-patch; > 0001-DRAFT-Fix-dependence-on-locale-in-org-testing-facili.patch]... > > > To summarize, the remaining goal of this patchset is to fix > `test-org-colview/columns-width', and to optionally expect-failure on > `test-org-clock/clocktable/lang'. Iteration v6. Everything, other than the French test and the batch-non-batch Chinese test, now passes. I'll look up how to determine whether we are in batch mode or not, and slap that onto the expect tag for `test-org-clock/clocktable/lang'. And the French test belongs to its own thread, so I won't try to fix it here. V6 has two commits: the first commit is the *exact copy* of v5, and the second commit tries to fix the zh_CN columns-width code & test. Hence I only attach the patch file for the second commit.
>From 59cbb93b9fc221bdc8ee708b05943a245c41ad25 Mon Sep 17 00:00:00 2001 From: Ruijie Yu <rui...@netyu.xyz> Date: Tue, 25 Apr 2023 22:56:02 +0800 Subject: [PATCH 2/2] Let org-columns correctly detect string-widths in code TODO: maybe I should also make a test directly on `org-columns-add-ellipses'. Will do in next iteration unless objections. * lisp/org-colview.el (org-columns--truncate-below-width): add a helper function that will trim off just enough data from string to fit into expected width. (org-columns-add-ellipses): make sure to do truncation correctly even in CJK locales (where an ellipsis character takes two spaces). * testing/lisp/test-org-colview.el (test-org-colview/substring-below-width): add test to make sure helper function is correct. (test-org-colview/columns-width): fix incorrect expectations for CJK locales about ellipses. --- lisp/org-colview.el | 26 +++++++++++++++++++++----- testing/lisp/test-org-colview.el | 19 ++++++++++++++++++- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/lisp/org-colview.el b/lisp/org-colview.el index 92a3b473d..6311a9274 100644 --- a/lisp/org-colview.el +++ b/lisp/org-colview.el @@ -452,14 +452,30 @@ DATELINE is non-nil when the face used should be "Type \\<org-columns-map>`\\[org-columns-edit-value]' \ to edit property"))))))) +(defun org-columns--truncate-below-width (string width) + "Return a substring of STRING no wider than WIDTH. +This substring must start at 0, and must be the longest possible +substring whose `string-width' does not exceed WIDTH." + (declare (side-effect-free t)) + (let ((end (min width (length string))) res) + (while (and end (>= end 0)) + (let* ((curr (string-width string 0 end)) + (excess (- curr width))) + (if (> excess 0) + (cl-decf end (max 1 (/ excess 2))) + (setq res (substring string 0 end) end nil)))) + res)) + (defun org-columns-add-ellipses (string width) "Truncate STRING with WIDTH characters, with ellipses." (cond - ((<= (length string) width) string) - ((<= width (length org-columns-ellipses)) - (substring org-columns-ellipses 0 width)) - (t (concat (substring string 0 (- width (length org-columns-ellipses))) - org-columns-ellipses)))) + ((<= (string-width string) width) string) + ((<= width (string-width org-columns-ellipses)) + (org-columns--truncate-below-width org-columns-ellipses width)) + (t (concat + (org-columns--truncate-below-width + string (- width (string-width org-columns-ellipses))) + org-columns-ellipses)))) (defvar org-columns-full-header-line-format nil "The full header line format, will be shifted by horizontal scrolling." ) diff --git a/testing/lisp/test-org-colview.el b/testing/lisp/test-org-colview.el index 9daec18e2..b421fdc41 100644 --- a/testing/lisp/test-org-colview.el +++ b/testing/lisp/test-org-colview.el @@ -26,6 +26,23 @@ (require 'org-duration) (require 'org-inlinetask) +(ert-deftest test-org-colview/substring-below-width () + "Test `org-columns--truncate-below-width'." + (cl-flet ((check (string width expect) + (string= expect (org-columns--truncate-below-width + string width)))) + (if (= (char-width ?…) 2) + (progn (should (check "12…" 3 "12")) + (should (check "1…2" 1 "1")) + (should (check "1…2" 2 "1")) + (should (check "1…2" 3 "1…")) + (should (check "……………………" 7 "………"))) + (progn (should (check "12…" 4 "12…")) + (should (check "1…2" 1 "1")) + (should (check "1…2" 2 "1…")) + (should (check "1…2" 3 "1…2")) + (should (check "……………………" 7 "…………………")))))) + (ert-deftest test-org-colview/get-format () "Test `org-columns-get-format' specifications." ;; Without any clue, use `org-columns-default-format'. @@ -160,7 +177,7 @@ (org-columns)) (org-trim (get-char-property (point) 'display))))) (should - (equal "1234… |" + (equal (if (= 1 (char-width ?…)) "1234… |" "123… |") (org-test-with-temp-text "* H\n:PROPERTIES:\n:P: 123456\n:END:" (let ((org-columns-default-format "%5P") (org-columns-ellipses "…")) -- 2.40.0
Comments welcome, thanks. -- Best, RY [Please note that this mail might go to spam due to some misconfiguration in my mail server -- still investigating.]