From e85a872f3bc64826ebc51b4dafadd485a276ff4b Mon Sep 17 00:00:00 2001 From: Kyle Meyer Date: Sun, 27 Feb 2022 23:31:49 -0500 Subject: [PATCH 1/4] test-org.el: Actually test org-copy-visible * testing/lisp/test-org.el (test-org/copy-visible): Switch defun to ert-deftest so that the test is actually executed, and resolve failures in test. Make two adjustments so that the now executed test passes: - Correct the end value of the hidden part of the string for one test case. - Disable org-unfontify-region so that the invisible properties show up in the temporary buffer. I think org-unfontify-region should probably be changed to _not_ remove the invisible property, but it's a longstanding behavior. Changing it would need more thought and isn't something that should be done on the bugfix branch. --- testing/lisp/test-org.el | 94 +++++++++++++++++++++------------------- 1 file changed, 49 insertions(+), 45 deletions(-) diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el index 5b161315d..57daf4eeb 100644 --- a/testing/lisp/test-org.el +++ b/testing/lisp/test-org.el @@ -8176,52 +8176,56 @@ CLOSED: %s (org-show-set-visibility 'minimal) (org-invisible-p2)))) -(defun test-org/copy-visible () +(ert-deftest test-org/copy-visible () "Test `org-copy-visible' specifications." - (should - (equal "Foo" - (org-test-with-temp-text "Foo" - (let ((kill-ring nil)) - (org-copy-visible (point-min) (point-max)) - (current-kill 0 t))))) - ;; Skip invisible characters by text property. - (should - (equal "Foo" - (org-test-with-temp-text #("Foo" 1 7 (invisible t)) - (let ((kill-ring nil)) - (org-copy-visible (point-min) (point-max)) - (current-kill 0 t))))) - ;; Skip invisible characters by overlay. - (should - (equal "Foo" - (org-test-with-temp-text "Foo" - (let ((o (make-overlay 2 10))) - (overlay-put o 'invisible t)) - (let ((kill-ring nil)) - (org-copy-visible (point-min) (point-max)) - (current-kill 0 t))))) - ;; Handle invisible characters at the beginning and the end of the - ;; buffer. - (should - (equal "Foo" - (org-test-with-temp-text #("Foo" 0 8 (invisible t)) - (let ((kill-ring nil)) - (org-copy-visible (point-min) (point-max)) - (current-kill 0 t))))) - (should - (equal "Foo" - (org-test-with-temp-text #("Foo" 3 11 (invisible t)) - (let ((kill-ring nil)) - (org-copy-visible (point-min) (point-max)) - (current-kill 0 t))))) - ;; Handle multiple visible parts. - (should - (equal "abc" - (org-test-with-temp-text - #("aXbXc" 1 2 (invisible t) 3 4 (invisible t)) - (let ((kill-ring nil)) - (org-copy-visible (point-min) (point-max)) - (current-kill 0 t)))))) + ;;`org-unfontify-region', which is wired up to + ;; `font-lock-unfontify-region-function', removes the invisible text + ;; property, among other things. + (cl-letf (((symbol-function 'org-unfontify-region) #'ignore)) + (should + (equal "Foo" + (org-test-with-temp-text "Foo" + (let ((kill-ring nil)) + (org-copy-visible (point-min) (point-max)) + (current-kill 0 t))))) + ;; Skip invisible characters by text property. + (should + (equal "Foo" + (org-test-with-temp-text #("Foo" 1 9 (invisible t)) + (let ((kill-ring nil)) + (org-copy-visible (point-min) (point-max)) + (current-kill 0 t))))) + ;; Skip invisible characters by overlay. + (should + (equal "Foo" + (org-test-with-temp-text "Foo" + (let ((o (make-overlay 2 10))) + (overlay-put o 'invisible t)) + (let ((kill-ring nil)) + (org-copy-visible (point-min) (point-max)) + (current-kill 0 t))))) + ;; Handle invisible characters at the beginning and the end of the + ;; buffer. + (should + (equal "Foo" + (org-test-with-temp-text #("Foo" 0 8 (invisible t)) + (let ((kill-ring nil)) + (org-copy-visible (point-min) (point-max)) + (current-kill 0 t))))) + (should + (equal "Foo" + (org-test-with-temp-text #("Foo" 3 11 (invisible t)) + (let ((kill-ring nil)) + (org-copy-visible (point-min) (point-max)) + (current-kill 0 t))))) + ;; Handle multiple visible parts. + (should + (equal "abc" + (org-test-with-temp-text + #("aXbXc" 1 2 (invisible t) 3 4 (invisible t)) + (let ((kill-ring nil)) + (org-copy-visible (point-min) (point-max)) + (current-kill 0 t))))))) (ert-deftest test-org/set-visibility-according-to-property () "Test `org-set-visibility-according-to-property' specifications." From f2833ff25508d49845a2bdb4c4e6d7b7ec062ce5 Mon Sep 17 00:00:00 2001 From: Kyle Meyer Date: Sun, 27 Feb 2022 23:31:49 -0500 Subject: [PATCH 2/4] org-copy-visible: Fix handling of adjacent invisible text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * lisp/org.el (org-copy-visible): Don't copy invisible text that follows invisible text with a different property value. If org-copy-visible sees that the left bound position has a non-nil invisible property, it uses next-single-char-property-change to find the new bound. However, next-single-char-property-change may just find a bound that still has a _different_ non-nil invisible property. Reported-by: "Максим Бабушкин" Link: https://debbugs.gnu.org/49967 --- lisp/org.el | 10 +++++----- testing/lisp/test-org.el | 8 ++++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/lisp/org.el b/lisp/org.el index 7ea8d65f3..4fcc2cd90 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -17522,11 +17522,11 @@ this numeric value." (interactive "r") (let ((result "")) (while (/= beg end) - (when (get-char-property beg 'invisible) - (setq beg (next-single-char-property-change beg 'invisible nil end))) - (let ((next (next-single-char-property-change beg 'invisible nil end))) - (setq result (concat result (buffer-substring beg next))) - (setq beg next))) + (if (get-char-property beg 'invisible) + (setq beg (next-single-char-property-change beg 'invisible nil end)) + (let ((next (next-single-char-property-change beg 'invisible nil end))) + (setq result (concat result (buffer-substring beg next))) + (setq beg next)))) (setq deactivate-mark t) (kill-new result) (message "Visible strings have been copied to the kill ring."))) diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el index 57daf4eeb..6e6a1f852 100644 --- a/testing/lisp/test-org.el +++ b/testing/lisp/test-org.el @@ -8223,6 +8223,14 @@ CLOSED: %s (equal "abc" (org-test-with-temp-text #("aXbXc" 1 2 (invisible t) 3 4 (invisible t)) + (let ((kill-ring nil)) + (org-copy-visible (point-min) (point-max)) + (current-kill 0 t))))) + ;; Handle adjacent invisible parts. + (should + (equal "ab" + (org-test-with-temp-text + #("aXXb" 1 2 (invisible t) 2 3 (invisible org-link)) (let ((kill-ring nil)) (org-copy-visible (point-min) (point-max)) (current-kill 0 t))))))) From 57362f741492253782e585901d82eedc0da23064 Mon Sep 17 00:00:00 2001 From: Kyle Meyer Date: Sun, 27 Feb 2022 23:31:49 -0500 Subject: [PATCH 3/4] org-copy-visible: Respect buffer-invisibility-spec * lisp/org.el (org-copy-visible): Decide whether text is invisible by calling invisible-p rather than checking whether the invisible property at point is non-nil. Text may have a non-nil invisible property but _not_ be hidden from the user (and thus should be copied by org-copy-visible). For example, the link itself is shown when org-link-descriptive is nil, but it still has an invisible property of `org-link'. --- lisp/org.el | 2 +- testing/lisp/test-org.el | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lisp/org.el b/lisp/org.el index 4fcc2cd90..d656a5159 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -17522,7 +17522,7 @@ this numeric value." (interactive "r") (let ((result "")) (while (/= beg end) - (if (get-char-property beg 'invisible) + (if (invisible-p beg) (setq beg (next-single-char-property-change beg 'invisible nil end)) (let ((next (next-single-char-property-change beg 'invisible nil end))) (setq result (concat result (buffer-substring beg next))) diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el index 6e6a1f852..d552474e2 100644 --- a/testing/lisp/test-org.el +++ b/testing/lisp/test-org.el @@ -8231,6 +8231,17 @@ CLOSED: %s (equal "ab" (org-test-with-temp-text #("aXXb" 1 2 (invisible t) 2 3 (invisible org-link)) + (let ((kill-ring nil)) + (org-copy-visible (point-min) (point-max)) + (current-kill 0 t))))) + ;; Copies text based on what's actually visible, as defined by + ;; `buffer-invisibility-spec'. + (should + (equal "aYb" + (org-test-with-temp-text + #("aXYb" + 1 2 (invisible t) + 2 3 (invisible org-test-copy-visible)) (let ((kill-ring nil)) (org-copy-visible (point-min) (point-max)) (current-kill 0 t))))))) From 33543d2aa87c994475646897b2d2cd40880e0fab Mon Sep 17 00:00:00 2001 From: Kyle Meyer Date: Sun, 27 Feb 2022 23:40:13 -0500 Subject: [PATCH 4/4] org-link-descriptive: Fix docstring typo * lisp/ol.el (org-link-descriptive): Drop extra word from docstring. --- lisp/ol.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lisp/ol.el b/lisp/ol.el index 2cba33ed9..a03d85f61 100644 --- a/lisp/ol.el +++ b/lisp/ol.el @@ -183,7 +183,7 @@ link. (defcustom org-link-descriptive t "Non-nil means Org displays descriptive links. -E.g. [[https://orgmode.org][Org website]] is be displayed as +E.g. [[https://orgmode.org][Org website]] is displayed as \"Org Website\", hiding the link itself and just displaying its description. When set to nil, Org displays the full links literally.