* 6 failing tests on master branch @ 2017-06-08 20:33 Kaushal Modi 2017-06-08 21:10 ` Nicolas Goaziou 0 siblings, 1 reply; 19+ messages in thread From: Kaushal Modi @ 2017-06-08 20:33 UTC (permalink / raw) To: emacs-org list [-- Attachment #1: Type: text/plain, Size: 2325 bytes --] Hello, I ran make test today using the latest version of org master branch (and latest of emacs master), and I see 6 failures: 6 unexpected results: FAILED ob-fortran/list-matrix-from-table1 FAILED ob-fortran/list-matrix-from-table2 FAILED ob-fortran/list-var-from-table FAILED test-org-export/file-uri FAILED test-org/custom-properties FAILED test-org/forward-paragraph I remember reading that someone else too saw the fortran errors.. Is it because I don't have fortran on my system? About the other 3 errors: Test test-org-export/file-uri condition: (ert-test-failed ((should (equal "file://myself@some.where:papers/last.pdf" (org-export-file-uri "/myself@some.where:papers/last.pdf"))) :form (equal "file://myself@some.where:papers/last.pdf" "file:///myself@some.where:papers/last.pdf") :value nil :explanation (arrays-of-different-length 40 41 "file://myself@some.where:papers/last.pdf" "file:///myself@some.where:papers/last.pdf" first-mismatch-at 7))) FAILED 406/734 test-org-export/file-uri Test test-org/custom-properties condition: (ert-test-failed ((should (let (...) (org-test-with-temp-text "* H :PROPERTIES: <point>:FOO: val :END: " ... ...))) :form (let ((org-custom-properties ...)) (org-test-with-temp-text "* H :PROPERTIES: <point>:FOO: val :END: " (org-toggle-custom-properties-visibility) (org-invisible-p2))) :value nil)) FAILED 646/734 test-org/custom-properties (ert-test-failed ((should (org-test-with-temp-text "#+BEGIN_CENTER P1 P2 #+END_CENTER P3" (org-hide-block-toggle) (org-forward-paragraph) (looking-at "P3"))) :form (let ((inside-text ...) (org-mode-hook nil)) (with-temp-buffer (org-mode) (let ... ...) (org-hide-block-toggle) (org-forward-paragraph) (looking-at "P3"))) :value nil)) FAILED 668/734 test-org/forward-paragraph --- I have an unrelated make test question too. If I add a test, say test-org/foo, in order to make learning ert and org test writing faster, it is possible to run just that one test-org/foo and not the whole suite? -- Kaushal Modi [-- Attachment #2: Type: text/html, Size: 3617 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: 6 failing tests on master branch 2017-06-08 20:33 6 failing tests on master branch Kaushal Modi @ 2017-06-08 21:10 ` Nicolas Goaziou 2017-06-09 14:26 ` Kaushal Modi 2017-06-14 14:56 ` Backward incompatible outline-invisible-p change in emacs master for Org (Was: 6 failing tests on master branch) Kaushal Modi 0 siblings, 2 replies; 19+ messages in thread From: Nicolas Goaziou @ 2017-06-08 21:10 UTC (permalink / raw) To: Kaushal Modi; +Cc: emacs-org list Hello, Kaushal Modi <kaushal.modi@gmail.com> writes: > 6 unexpected results: > FAILED ob-fortran/list-matrix-from-table1 > FAILED ob-fortran/list-matrix-from-table2 > FAILED ob-fortran/list-var-from-table > FAILED test-org-export/file-uri > FAILED test-org/custom-properties > FAILED test-org/forward-paragraph > > I remember reading that someone else too saw the fortran errors.. Is it > because I don't have fortran on my system? No, I don't think so. OTOH, I have no idea about where this comes from. > About the other 3 errors: Since our buildbot doesn't report any problem, this seems specific to Emacs master. > Test test-org-export/file-uri condition: > (ert-test-failed > ((should > (equal "file://myself@some.where:papers/last.pdf" > (org-export-file-uri "/myself@some.where:papers/last.pdf"))) > :form > (equal "file://myself@some.where:papers/last.pdf" > "file:///myself@some.where:papers/last.pdf") > :value nil :explanation > (arrays-of-different-length 40 41 > "file://myself@some.where:papers/last.pdf" > "file:///myself@some.where:papers/last.pdf" first-mismatch-at 7))) This one should be easy to debug since (org-export-file-uri "/myself@some.where:papers/last.pdf") is easy to reproduce. Could you investigate where the spurious "/" comes from? > Test test-org/custom-properties condition: > (ert-test-failed > ((should > (let > (...) > (org-test-with-temp-text "* H > :PROPERTIES: > <point>:FOO: val > :END: > " ... ...))) > :form > (let > ((org-custom-properties ...)) > (org-test-with-temp-text "* H > :PROPERTIES: > <point>:FOO: val > :END: > " > (org-toggle-custom-properties-visibility) > (org-invisible-p2))) > :value nil)) > FAILED 646/734 test-org/custom-properties > > (ert-test-failed > ((should > (org-test-with-temp-text "#+BEGIN_CENTER > P1 > > P2 > #+END_CENTER > P3" > (org-hide-block-toggle) > (org-forward-paragraph) > (looking-at "P3"))) > :form > (let > ((inside-text ...) > (org-mode-hook nil)) > (with-temp-buffer > (org-mode) > (let ... ...) > (org-hide-block-toggle) > (org-forward-paragraph) > (looking-at "P3"))) > :value nil)) > FAILED 668/734 test-org/forward-paragraph These one are related to invisible text. I don't what is going to change in this area in next Emacs release. > I have an unrelated make test question too. > > If I add a test, say test-org/foo, in order to make learning ert and org > test writing faster, it is possible to run just that one test-org/foo and > not the whole suite? You can run, e.g., BTEST_RE="foo" make test Regards, -- Nicolas Goaziou ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: 6 failing tests on master branch 2017-06-08 21:10 ` Nicolas Goaziou @ 2017-06-09 14:26 ` Kaushal Modi 2017-06-09 15:14 ` Kaushal Modi 2017-06-14 14:56 ` Backward incompatible outline-invisible-p change in emacs master for Org (Was: 6 failing tests on master branch) Kaushal Modi 1 sibling, 1 reply; 19+ messages in thread From: Kaushal Modi @ 2017-06-09 14:26 UTC (permalink / raw) To: Nicolas Goaziou; +Cc: emacs-org list [-- Attachment #1: Type: text/plain, Size: 2938 bytes --] On Thu, Jun 8, 2017 at 5:10 PM Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote: > > I remember reading that someone else too saw the fortran errors.. Is it > > because I don't have fortran on my system? > > No, I don't think so. OTOH, I have no idea about where this comes from. > Did little digging, TIL that the gfortran binary is part of GCC.. as gfortran binary. So I do have fortran, but I never use it. Also found out why fortran tests were failing for me.. by default the gcc version on my system (RHEL 6.6) is 4.4.7. Using GCC 6.1.0 passes the full "make test" on emacs 25.2. For example, this is one of the ob-fortran examples that was failing: * matrix :PROPERTIES: :ID: 3f73ab19-d25a-428d-8c26-e8c6aa933976 :END: Real matrix as input #+name: fortran-input-matrix1 | 0.0 | 42.0 | | 0.0 | 0.0 | | 0.0 | 0.0 | #+name: fortran-input-matrix2 | 0.0 | 0.0 | 0.0 | | 0.0 | 0.0 | 42.0 | #+begin_src fortran :var s=fortran-input-matrix1 :results silent write (*, '(i2)'), nint(s(1,2)) #+end_src #+begin_src fortran :var s=fortran-input-matrix2 :results silent write (*, '(i2)'), nint(s(2,3)) #+end_src Doing C-c C-c on the first source block gives this error with gcc 4.4.7: /tmp/babel-6872M6V/fortran-src-6872rcR.F90:4.27: real, parameter :: s(3,2) = transpose( reshape( (/(/0.0, 42.0/), (/0.0, 0.0/), 1 Error: transformational intrinsic 'transpose' at (1) is not permitted in an initialization expression /bin/sh: /tmp/babel-6872M6V/fortran-bin-68724mX: Permission denied That expression is apparently legal in newer fortran, is my guess. > > About the other 3 errors: > > Since our buildbot doesn't report any problem, this seems specific to > Emacs master. > Thanks. I confirm that make test on org master passes with emacs 25.2. > Test test-org-export/file-uri condition: > > (ert-test-failed > > ((should > > (equal "file://myself@some.where:papers/last.pdf" > > (org-export-file-uri "/myself@some.where > :papers/last.pdf"))) > > :form > > (equal "file://myself@some.where:papers/last.pdf" > > "file:///myself@some.where:papers/last.pdf") > > :value nil :explanation > > (arrays-of-different-length 40 41 > > "file://myself@some.where:papers/last.pdf" > > "file:///myself@some.where:papers/last.pdf" first-mismatch-at 7))) > > This one should be easy to debug since > > (org-export-file-uri "/myself@some.where:papers/last.pdf") > > is easy to reproduce. Could you investigate where the spurious "/" comes > from? > Will do. > > FAILED 646/734 test-org/custom-properties > > FAILED 668/734 test-org/forward-paragraph > > These one are related to invisible text. I don't what is going to change > in this area in next Emacs release. > I'll have to leave this for a future debug.. Someone else wants to help out with this? > You can run, e.g., > > BTEST_RE="foo" make test > Thanks! -- Kaushal Modi [-- Attachment #2: Type: text/html, Size: 4680 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: 6 failing tests on master branch 2017-06-09 14:26 ` Kaushal Modi @ 2017-06-09 15:14 ` Kaushal Modi 2017-06-09 16:02 ` Michael Albinus 0 siblings, 1 reply; 19+ messages in thread From: Kaushal Modi @ 2017-06-09 15:14 UTC (permalink / raw) To: Nicolas Goaziou; +Cc: Michael Albinus, emacs-org list [-- Attachment #1: Type: text/plain, Size: 1010 bytes --] On Fri, Jun 9, 2017 at 10:26 AM Kaushal Modi <kaushal.modi@gmail.com> wrote: > This one should be easy to debug since >> >> (org-export-file-uri "/myself@some.where:papers/last.pdf") >> >> is easy to reproduce. Could you investigate where the spurious "/" comes >> from? >> > > Will do. > This seems to be related to a tramp file notation change on emacs master. I don't use tramp, but I heard about it on emacs-devel. Here is a test expression: (find-file-name-handler "/myself@some.where:papers/last.pdf" 'file-remote-p) On emacs 25.2, that returns tramp-file-name-handler. On emacs master, that returns nil. So, on emacs 25.2, in org-export-file-uri, the (org-file-remote-p filename) case evaluates to true. Whereas, on emacs master, the execution falls into the default (t ..) case. .. and thus the spurious "/". Copying the tramp dev Michael Albinus on this for help. @Michael How should the above find-file-name-handler arg change to support that test file path? Thanks. -- Kaushal Modi [-- Attachment #2: Type: text/html, Size: 1850 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: 6 failing tests on master branch 2017-06-09 15:14 ` Kaushal Modi @ 2017-06-09 16:02 ` Michael Albinus 2017-06-09 16:10 ` Kaushal Modi 0 siblings, 1 reply; 19+ messages in thread From: Michael Albinus @ 2017-06-09 16:02 UTC (permalink / raw) To: Kaushal Modi; +Cc: emacs-org list, Nicolas Goaziou Kaushal Modi <kaushal.modi@gmail.com> writes: > This seems to be related to a tramp file notation change on emacs > master. I don't use tramp, but I heard about it on emacs-devel. Indeed. > Here is a test expression: > > (find-file-name-handler "/myself@some.where:papers/last.pdf" > 'file-remote-p) > > On emacs 25.2, that returns tramp-file-name-handler. > On emacs master, that returns nil. > > @Michael How should the above find-file-name-handler arg change to > support that test file path? The stronger Tramp file name syntax requires now a method. Something like "/ssh:myself@some.where:papers/last.pdf". This is backward compatible with Emacs 25, 24, etc pp. > Thanks. Best regards, Michael. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: 6 failing tests on master branch 2017-06-09 16:02 ` Michael Albinus @ 2017-06-09 16:10 ` Kaushal Modi 2017-06-09 16:27 ` Nicolas Goaziou 0 siblings, 1 reply; 19+ messages in thread From: Kaushal Modi @ 2017-06-09 16:10 UTC (permalink / raw) To: Michael Albinus, Nicolas Goaziou; +Cc: emacs-org list [-- Attachment #1: Type: text/plain, Size: 543 bytes --] On Fri, Jun 9, 2017 at 12:02 PM Michael Albinus <michael.albinus@gmx.de> wrote: > Kaushal Modi <kaushal.modi@gmail.com> writes: > > The stronger Tramp file name syntax requires now a method. Something > like "/ssh:myself@some.where:papers/last.pdf". This is backward > compatible with Emacs 25, 24, etc pp. > Thanks for the quick reply! @Nicholas Does org-export-file-uri need to change? Because now (org-export-file-uri "/ssh:myself@some.where:papers/last.pdf") returns "file://ssh:myself@some.where:papers/last.pdf" -- Kaushal Modi [-- Attachment #2: Type: text/html, Size: 1130 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: 6 failing tests on master branch 2017-06-09 16:10 ` Kaushal Modi @ 2017-06-09 16:27 ` Nicolas Goaziou 2017-06-09 19:55 ` Kaushal Modi 0 siblings, 1 reply; 19+ messages in thread From: Nicolas Goaziou @ 2017-06-09 16:27 UTC (permalink / raw) To: Kaushal Modi; +Cc: emacs-org list, Michael Albinus Hello, Kaushal Modi <kaushal.modi@gmail.com> writes: > Does org-export-file-uri need to change? Because now > > (org-export-file-uri "/ssh:myself@some.where:papers/last.pdf") > > returns > > "file://ssh:myself@some.where:papers/last.pdf" AFAIU, the function doesn't need to change, but the manual needs to be updated since, in (info "(org) External links") there is: /myself@some.where:papers/last.pdf same as above which is no longer a valid syntax. Regards, -- Nicolas Goaziou 0x80A93738 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: 6 failing tests on master branch 2017-06-09 16:27 ` Nicolas Goaziou @ 2017-06-09 19:55 ` Kaushal Modi 2017-06-09 20:05 ` Nicolas Goaziou 0 siblings, 1 reply; 19+ messages in thread From: Kaushal Modi @ 2017-06-09 19:55 UTC (permalink / raw) To: Nicolas Goaziou; +Cc: emacs-org list, Michael Albinus [-- Attachment #1.1: Type: text/plain, Size: 552 bytes --] On Fri, Jun 9, 2017 at 12:27 PM Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote: > AFAIU, the function doesn't need to change, but the manual needs to be > updated since, in (info "(org) External links") there is: > > /myself@some.where:papers/last.pdf same as above > > which is no longer a valid syntax. > Here's a patch rebased off maint (as it's just test and manual change with edits to a backward compatible Tramp syntax). There were some alignment issues in that External links section. So I have fixed that too. -- Kaushal Modi [-- Attachment #1.2: Type: text/html, Size: 972 bytes --] [-- Attachment #2: 0001-Update-remote-file-syntax-for-Tramp.patch --] [-- Type: application/octet-stream, Size: 8255 bytes --] From d61d6f9543bda7b4078d7a74e2a3c38eada0e44e Mon Sep 17 00:00:00 2001 From: Kaushal Modi <kaushal.modi@gmail.com> Date: Fri, 9 Jun 2017 15:28:49 -0400 Subject: [PATCH] Update remote file syntax for Tramp * testing/lisp/test-ox.el (test-org-export/file-uri): * doc/org.texi (External links): The method part of remote file names is mandatory now in the emacs master (26+). A valid remote file name starts with "/method:host:" or "/method:user@host:". ssh is used as an example method here. This change in the examples is backward compatible with emacs 25, 24, .. Suggested by: Michael Albinus <michael.albinus@gmx.de> <http://lists.gnu.org/archive/html/emacs-orgmode/2017-06/msg00151.html> Relevant commit in emacs master causing this change: <http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=ed33337c3e0d0b1a8b140e23168421ea43d79324> --- doc/org.texi | 80 ++++++++++++++++++++++++------------------------- testing/lisp/test-ox.el | 4 +-- 2 files changed, 42 insertions(+), 42 deletions(-) diff --git a/doc/org.texi b/doc/org.texi index c50e44c3ad..93a4b2e484 100644 --- a/doc/org.texi +++ b/doc/org.texi @@ -3531,44 +3531,44 @@ string followed by a colon. There can be no space after the colon. The following list shows examples for each link type. @example -http://www.astro.uva.nl/~dominik @r{on the web} -doi:10.1000/182 @r{DOI for an electronic resource} -file:/home/dominik/images/jupiter.jpg @r{file, absolute path} -/home/dominik/images/jupiter.jpg @r{same as above} -file:papers/last.pdf @r{file, relative path} -./papers/last.pdf @r{same as above} -file:/myself@@some.where:papers/last.pdf @r{file, path on remote machine} -/myself@@some.where:papers/last.pdf @r{same as above} -file:sometextfile::NNN @r{file, jump to line number} -file:projects.org @r{another Org file} -file:projects.org::some words @r{text search in Org file}@footnote{ +http://www.astro.uva.nl/~dominik @r{on the web} +doi:10.1000/182 @r{DOI for an electronic resource} +file:/home/dominik/images/jupiter.jpg @r{file, absolute path} +/home/dominik/images/jupiter.jpg @r{same as above} +file:papers/last.pdf @r{file, relative path} +./papers/last.pdf @r{same as above} +file:/ssh:myself@@some.where:papers/last.pdf @r{file, path on remote machine} +/ssh:myself@@some.where:papers/last.pdf @r{same as above} +file:sometextfile::NNN @r{file, jump to line number} +file:projects.org @r{another Org file} +file:projects.org::some words @r{text search in Org file}@footnote{ The actual behavior of the search will depend on the value of the option @code{org-link-search-must-match-exact-headline}. If its value -is @code{nil}, then a fuzzy text search will be done. If it is t, then only the -exact headline will be matched, ignoring spaces and cookies. If the value is -@code{query-to-create}, then an exact headline will be searched; if it is not -found, then the user will be queried to create it.} -file:projects.org::*task title @r{heading search in Org -file}@footnote{Headline searches always match the exact headline, ignoring +is @code{nil}, then a fuzzy text search will be done. If it is t, then only +the exact headline will be matched, ignoring spaces and cookies. If the +value is @code{query-to-create}, then an exact headline will be searched; if +it is not found, then the user will be queried to create it.} +file:projects.org::*task title @r{heading search in Org file}@footnote{ +Headline searches always match the exact headline, ignoring spaces and cookies. If the headline is not found and the value of the option @code{org-link-search-must-match-exact-headline} is @code{query-to-create}, then the user will be queried to create it.} -docview:papers/last.pdf::NNN @r{open in doc-view mode at page} -id:B7423F4D-2E8A-471B-8810-C40F074717E9 @r{Link to heading by ID} -news:comp.emacs @r{Usenet link} -mailto:adent@@galaxy.net @r{Mail link} -mhe:folder @r{MH-E folder link} -mhe:folder#id @r{MH-E message link} -rmail:folder @r{RMAIL folder link} -rmail:folder#id @r{RMAIL message link} -gnus:group @r{Gnus group link} -gnus:group#id @r{Gnus article link} -bbdb:R.*Stallman @r{BBDB link (with regexp)} -irc:/irc.com/#emacs/bob @r{IRC link} -info:org#External links @r{Info node or index link} -shell:ls *.org @r{A shell command} -elisp:org-agenda @r{Interactive Elisp command} -elisp:(find-file-other-frame "Elisp.org") @r{Elisp form to evaluate} +docview:papers/last.pdf::NNN @r{open in doc-view mode at page} +id:B7423F4D-2E8A-471B-8810-C40F074717E9 @r{Link to heading by ID} +news:comp.emacs @r{Usenet link} +mailto:adent@@galaxy.net @r{Mail link} +mhe:folder @r{MH-E folder link} +mhe:folder#id @r{MH-E message link} +rmail:folder @r{RMAIL folder link} +rmail:folder#id @r{RMAIL message link} +gnus:group @r{Gnus group link} +gnus:group#id @r{Gnus article link} +bbdb:R.*Stallman @r{BBDB link (with regexp)} +irc:/irc.com/#emacs/bob @r{IRC link} +info:org#External links @r{Info node or index link} +shell:ls *.org @r{A shell command} +elisp:org-agenda @r{Interactive Elisp command} +elisp:(find-file-other-frame "Elisp.org") @r{Elisp form to evaluate} @end example @cindex VM links @@ -3579,13 +3579,13 @@ to VM or Wanderlust messages are available when you load the corresponding libraries from the @code{contrib/} directory: @example -vm:folder @r{VM folder link} -vm:folder#id @r{VM message link} -vm://myself@@some.where.org/folder#id @r{VM on remote machine} -vm-imap:account:folder @r{VM IMAP folder link} -vm-imap:account:folder#id @r{VM IMAP message link} -wl:folder @r{WANDERLUST folder link} -wl:folder#id @r{WANDERLUST message link} +vm:folder @r{VM folder link} +vm:folder#id @r{VM message link} +vm://myself@@some.where.org/folder#id @r{VM on remote machine} +vm-imap:account:folder @r{VM IMAP folder link} +vm-imap:account:folder#id @r{VM IMAP message link} +wl:folder @r{WANDERLUST folder link} +wl:folder#id @r{WANDERLUST message link} @end example For customizing Org to add new link types @ref{Adding hyperlink types}. diff --git a/testing/lisp/test-ox.el b/testing/lisp/test-ox.el index d69660178d..f55cdd4fec 100644 --- a/testing/lisp/test-ox.el +++ b/testing/lisp/test-ox.el @@ -3252,8 +3252,8 @@ Another text. (ref:text) (should (equal (concat (if (memq system-type '(windows-nt cygwin)) "file:///" "file://") (expand-file-name "/local.org")) (org-export-file-uri "/local.org"))) ;; Remote files start with "file://" - (should (equal "file://myself@some.where:papers/last.pdf" - (org-export-file-uri "/myself@some.where:papers/last.pdf"))) + (should (equal "file://ssh:myself@some.where:papers/last.pdf" + (org-export-file-uri "/ssh:myself@some.where:papers/last.pdf"))) (should (equal "file://localhost/etc/fstab" (org-export-file-uri "//localhost/etc/fstab"))) ;; Expand filename starting with "~". -- 2.13.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: 6 failing tests on master branch 2017-06-09 19:55 ` Kaushal Modi @ 2017-06-09 20:05 ` Nicolas Goaziou 2017-06-09 20:21 ` Kaushal Modi 0 siblings, 1 reply; 19+ messages in thread From: Nicolas Goaziou @ 2017-06-09 20:05 UTC (permalink / raw) To: Kaushal Modi; +Cc: emacs-org list, Michael Albinus Hello, Kaushal Modi <kaushal.modi@gmail.com> writes: > Here's a patch rebased off maint (as it's just test and manual change with > edits to a backward compatible Tramp syntax). LGTM. Thank you. Regards, -- Nicolas Goaziou 0x80A93738 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: 6 failing tests on master branch 2017-06-09 20:05 ` Nicolas Goaziou @ 2017-06-09 20:21 ` Kaushal Modi 0 siblings, 0 replies; 19+ messages in thread From: Kaushal Modi @ 2017-06-09 20:21 UTC (permalink / raw) To: Nicolas Goaziou; +Cc: emacs-org list, Michael Albinus [-- Attachment #1: Type: text/plain, Size: 361 bytes --] On Fri, Jun 9, 2017 at 4:05 PM Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote: > Hello, > > Kaushal Modi <kaushal.modi@gmail.com> writes: > > > Here's a patch rebased off maint (as it's just test and manual change > with > > edits to a backward compatible Tramp syntax). > > LGTM. > > Thank you. > Thanks, pushed to maint, merged to master. -- Kaushal Modi [-- Attachment #2: Type: text/html, Size: 822 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Backward incompatible outline-invisible-p change in emacs master for Org (Was: 6 failing tests on master branch) 2017-06-08 21:10 ` Nicolas Goaziou 2017-06-09 14:26 ` Kaushal Modi @ 2017-06-14 14:56 ` Kaushal Modi 2017-06-14 16:03 ` Kaushal Modi 2017-06-14 17:02 ` Backward incompatible outline-invisible-p change in emacs master for Org Bastien Guerry 1 sibling, 2 replies; 19+ messages in thread From: Kaushal Modi @ 2017-06-14 14:56 UTC (permalink / raw) To: Nicolas Goaziou, Bastien Guerry, Paul Rankin; +Cc: emacs-org list [-- Attachment #1: Type: text/plain, Size: 5102 bytes --] On Thu, Jun 8, 2017 at 5:10 PM Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote: > > Test test-org/custom-properties condition: > > (ert-test-failed > > ((should > > (let > > (...) > > (org-test-with-temp-text "* H > > :PROPERTIES: > > <point>:FOO: val > > :END: > > " ... ...))) > > :form > > (let > > ((org-custom-properties ...)) > > (org-test-with-temp-text "* H > > :PROPERTIES: > > <point>:FOO: val > > :END: > > " > > (org-toggle-custom-properties-visibility) > > (org-invisible-p2))) > > :value nil)) > > FAILED 646/734 test-org/custom-properties > > > > (ert-test-failed > > ((should > > (org-test-with-temp-text "#+BEGIN_CENTER > > P1 > > > > P2 > > #+END_CENTER > > P3" > > (org-hide-block-toggle) > > (org-forward-paragraph) > > (looking-at "P3"))) > > :form > > (let > > ((inside-text ...) > > (org-mode-hook nil)) > > (with-temp-buffer > > (org-mode) > > (let ... ...) > > (org-hide-block-toggle) > > (org-forward-paragraph) > > (looking-at "P3"))) > > :value nil)) > > FAILED 668/734 test-org/forward-paragraph > > These one are related to invisible text. I don't what is going to change > in this area in next Emacs release. > This commit in emacs master is causing this failure: http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=9cc59ffbbb2f20fbbf1c72d2e0c9dc47c7906a99 Earlier outline-invisible-p simply returned the value of (get-char-property (or pos (point)) 'invisible). But now (in emacs master), it does a check if that value is equal to 'outline. That test fails as (get-char-property (or pos (point)) 'invisible) returns 'org-hide-block, not 'outline. Now outline-invisible-p is used at many places! lisp/org-archive.el 227: cl (if (outline-invisible-p) (org-end-of-subtree nil t)))) 425: cl (if (outline-invisible-p) (org-end-of-subtree nil t)))) 573: cl (if (outline-invisible-p) (org-end-of-subtree nil t)))) 594: cl (if (outline-invisible-p) (org-end-of-subtree nil t)))) lisp/org-clock.el 2965: (when (outline-invisible-p) (org-show-context)))))) lisp/org.el 6926: (when (outline-invisible-p) (org-flag-heading nil)))) 6960: (when (outline-invisible-p) (org-flag-heading nil))) 7134: (when (and (not (outline-invisible-p)) 7136: (goto-char (point-at-eol)) (outline-invisible-p))) 7489: (when (or (outline-invisible-p) (org-invisible-p2)) 7530: (when (outline-invisible-p) 8387: (setq folded (outline-invisible-p))) 8483: (setq folded (outline-invisible-p))) 8536: (let* ((visp (not (outline-invisible-p))) 8593: (when (and (outline-invisible-p) visp) 10441: (when (outline-invisible-p) (org-show-context))) 11144: (when (or (outline-invisible-p) (org-invisible-p2)) (org-show-context 'mark-goto)))) 12395: cl (when (outline-invisible-p) (org-end-of-subtree nil t)))) 13323: (lambda () (when (outline-invisible-p) (org-end-of-subtree nil t)))) 13340: (lambda () (when (outline-invisible-p) (org-end-of-subtree nil t)))) 14887: '(when (outline-invisible-p) (org-end-of-subtree nil t)))) 23850: ;; Early versions of noutline don't have `outline-invisible-p'. 23852: (outline-invisible-p))) 23858: ;; Early versions of noutline don't have `outline-invisible-p'. 23859: (outline-invisible-p))) 24129: (not (outline-invisible-p 24265: ((outline-invisible-p (line-end-position)) 24352: (when (outline-invisible-p (point)) (beginning-of-visual-line)))) 24429: (when (outline-invisible-p (line-end-position)) (org-cycle)) 24910: (or (outline-invisible-p) 24912: (outline-invisible-p))) 24918: (outline-invisible-p)) lisp/org-list.el 158:(declare-function outline-invisible-p "outline" (&optional pos)) 2260: (outline-invisible-p))) lisp/org-crypt.el 179: (let ((folded (outline-invisible-p)) 207: (outline-invisible-p)))) contrib/lisp/org-drill.el 1458:;; (when (and (not (outline-invisible-p)) 1483: (when (and (not (outline-invisible-p)) One solution would be to have a new function "org-invisible-p" that restores the old definition of outline-invisible-p. Interestingly I find that an "org-invisible-p" did exist back then, but it was replaced with outline-invisible-p in 2011 ( http://orgmode.org/cgit.cgi/org-mode.git/commit/?id=84d7165d74a5061413168af435d61453be217933 ). Looks like that might need reverting. I also find an org-invisible-p2 function, but it's not clear why it does a (backward-char 1) before doing invisibility check.. and that too eventually relies on outline-visible-p. (Turns out this function has a long history: http://orgmode.org/cgit.cgi/org-mode.git/commit/?id=93a4128a6fe47b4e05c0a7cb3ec14878b41d6000 ) I am copying Bastien and Paul as they would know the history behind the above referenced emacs master commit. -- Kaushal Modi [-- Attachment #2: Type: text/html, Size: 7195 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Backward incompatible outline-invisible-p change in emacs master for Org (Was: 6 failing tests on master branch) 2017-06-14 14:56 ` Backward incompatible outline-invisible-p change in emacs master for Org (Was: 6 failing tests on master branch) Kaushal Modi @ 2017-06-14 16:03 ` Kaushal Modi 2017-06-14 17:02 ` Backward incompatible outline-invisible-p change in emacs master for Org Bastien Guerry 1 sibling, 0 replies; 19+ messages in thread From: Kaushal Modi @ 2017-06-14 16:03 UTC (permalink / raw) To: Nicolas Goaziou, Bastien Guerry, Paul Rankin; +Cc: emacs-org list [-- Attachment #1.1: Type: text/plain, Size: 644 bytes --] On Wed, Jun 14, 2017 at 10:56 AM Kaushal Modi <kaushal.modi@gmail.com> wrote: > One solution would be to have a new function "org-invisible-p" that > restores the old definition of outline-invisible-p. Interestingly I find > that an "org-invisible-p" did exist back then, but it was replaced with > outline-invisible-p in 2011 ( > http://orgmode.org/cgit.cgi/org-mode.git/commit/?id=84d7165d74a5061413168af435d61453be217933 > ). Looks like that might need reverting. > Here's a patch for the above proposed solution. With that applied, 'make test' now passes cleanly on emacs master. Running 'make single' also looks good. -- Kaushal Modi [-- Attachment #1.2: Type: text/html, Size: 1243 bytes --] [-- Attachment #2: 0001-Fix-breakage-due-to-outline-invisible-p-defn-change-.patch --] [-- Type: application/octet-stream, Size: 18505 bytes --] From 0548946c6fc45355813fc2219f94205c4f103d96 Mon Sep 17 00:00:00 2001 From: Kaushal Modi <kaushal.modi@gmail.com> Date: Wed, 14 Jun 2017 11:20:05 -0400 Subject: [PATCH] Fix breakage due to outline-invisible-p defn change in emacs 26+ * lisp/org.el (org-invisible-p): New function. Restore the behavior of outline-invisible-p prior to the following commint on emacs master <http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=9cc59ffbbb2f20fbbf1c72d2e0c9dc47c7906a99>. * lisp/org.el (org-cycle-internal-local) (org-clean-visibility-after-subtree-move, org-goto) (org-get-location, org-move-subtree-down, org-copy-subtree) (org-paste-subtree, org-next-link, org-mark-ring-goto) (org-todo, org-deadline, org-schedule, org-set-tags) (org-truely-invisible-p, org-invisible-p2) (org-forward-heading-same-level, org-forward-paragraph) (org-backward-paragraph, org-down-element) (org-bookmark-jump-unhide, org-mark-jump-unhide): * lisp/org-list.el (org-insert-item): * lisp/org-crypt.el (org-encrypt-entry, org-decrypt-entry): * lisp/org-clock.el (org-clock-load): * lisp/org-archive.el (org-archive-subtree) (org-archive-to-archive-sibling, org-toggle-archive-tag) (org-archive-set-tag): * contrib/lisp/org-drill.el (org-drill-hide-subheadings-if): Use org-invisible-p instead of outline-invisible-p. Reference: <http://lists.gnu.org/archive/html/emacs-orgmode/2017-06/msg00230.html> --- contrib/lisp/org-drill.el | 4 +-- lisp/org-archive.el | 32 ++++++++++++----------- lisp/org-clock.el | 2 +- lisp/org-crypt.el | 4 +-- lisp/org-list.el | 4 +-- lisp/org.el | 65 ++++++++++++++++++++++++++--------------------- 6 files changed, 60 insertions(+), 51 deletions(-) diff --git a/contrib/lisp/org-drill.el b/contrib/lisp/org-drill.el index a78b8068ce..fb578ab863 100644 --- a/contrib/lisp/org-drill.el +++ b/contrib/lisp/org-drill.el @@ -1455,7 +1455,7 @@ How well did you do? %s" ;; (save-excursion ;; (org-map-entries ;; (lambda () -;; (when (and (not (outline-invisible-p)) +;; (when (and (not (org-invisible-p)) ;; (> (org-current-level) drill-entry-level)) ;; (setq drill-heading (org-get-heading t)) ;; (unless (and (= (org-current-level) (1+ drill-entry-level)) @@ -1480,7 +1480,7 @@ the current topic." (save-excursion (org-map-entries (lambda () - (when (and (not (outline-invisible-p)) + (when (and (not (org-invisible-p)) (> (org-current-level) drill-entry-level)) (when (or (/= (org-current-level) (1+ drill-entry-level)) (funcall test)) diff --git a/lisp/org-archive.el b/lisp/org-archive.el index c76abdb765..adb922e758 100644 --- a/lisp/org-archive.el +++ b/lisp/org-archive.el @@ -204,17 +204,19 @@ if LOCATION is not given, the value of `org-archive-location' is used." ;;;###autoload (defun org-archive-subtree (&optional find-done) "Move the current subtree to the archive. -The archive can be a certain top-level heading in the current file, or in -a different file. The tree will be moved to that location, the subtree -heading be marked DONE, and the current time will be added. - -When called with a single prefix argument FIND-DONE, find whole trees without any -open TODO items and archive them (after getting confirmation from the user). -When called with a double prefix argument, find whole trees with timestamps before -today and archive them (after getting confirmation from the user). -If the cursor is not at a headline when these commands are called, try all level -1 trees. If the cursor is on a headline, only try the direct children of -this heading." +The archive can be a certain top-level heading in the current +file, or in a different file. The tree will be moved to that +location, the subtree heading be marked DONE, and the current +time will be added. + +When called with a single prefix argument FIND-DONE, find whole +trees without any open TODO items and archive them (after getting +confirmation from the user). When called with a double prefix +argument, find whole trees with timestamps before today and +archive them (after getting confirmation from the user). If the +cursor is not at a headline when these commands are called, try +all level 1 trees. If the cursor is on a headline, only try the +direct children of this heading." (interactive "P") (if (and (org-region-active-p) org-loop-over-headlines-in-active-region) (let ((cl (if (eq org-loop-over-headlines-in-active-region 'start-level) @@ -224,7 +226,7 @@ this heading." `(progn (setq org-map-continue-from (progn (org-back-to-heading) (point))) (org-archive-subtree ,find-done)) org-loop-over-headlines-in-active-region - cl (if (outline-invisible-p) (org-end-of-subtree nil t)))) + cl (if (org-invisible-p) (org-end-of-subtree nil t)))) (cond ((equal find-done '(4)) (org-archive-all-done)) ((equal find-done '(16)) (org-archive-all-old)) @@ -422,7 +424,7 @@ Archiving time is retained in the ARCHIVE_TIME node property." (when (org-at-heading-p) (org-archive-to-archive-sibling))) org-loop-over-headlines-in-active-region - cl (if (outline-invisible-p) (org-end-of-subtree nil t)))) + cl (if (org-invisible-p) (org-end-of-subtree nil t)))) (save-restriction (widen) (let (b e pos leader level) @@ -570,7 +572,7 @@ the children that do not contain any open TODO items." (org-map-entries `(org-toggle-archive-tag ,find-done) org-loop-over-headlines-in-active-region - cl (if (outline-invisible-p) (org-end-of-subtree nil t)))) + cl (if (org-invisible-p) (org-end-of-subtree nil t)))) (if find-done (org-archive-all-done 'tag) (let (set) @@ -591,7 +593,7 @@ the children that do not contain any open TODO items." (org-map-entries 'org-archive-set-tag org-loop-over-headlines-in-active-region - cl (if (outline-invisible-p) (org-end-of-subtree nil t)))) + cl (if (org-invisible-p) (org-end-of-subtree nil t)))) (org-toggle-tag org-archive-tag 'on))) ;;;###autoload diff --git a/lisp/org-clock.el b/lisp/org-clock.el index e1a83ea8d6..d6dada7f0a 100644 --- a/lisp/org-clock.el +++ b/lisp/org-clock.el @@ -2962,7 +2962,7 @@ The details of what will be saved are regulated by the variable (let ((org-clock-in-resume 'auto-restart) (org-clock-auto-clock-resolution nil)) (org-clock-in) - (when (outline-invisible-p) (org-show-context)))))) + (when (org-invisible-p) (org-show-context)))))) (_ nil))))) ;; Suggested bindings diff --git a/lisp/org-crypt.el b/lisp/org-crypt.el index 4439c947c0..3c431e4fdd 100644 --- a/lisp/org-crypt.el +++ b/lisp/org-crypt.el @@ -176,7 +176,7 @@ See `org-crypt-disable-auto-save'." (let ((start-heading (point))) (org-end-of-meta-data) (unless (looking-at-p "-----BEGIN PGP MESSAGE-----") - (let ((folded (outline-invisible-p)) + (let ((folded (org-invisible-p)) (crypt-key (org-crypt-key-for-heading)) (beg (point))) (goto-char start-heading) @@ -204,7 +204,7 @@ See `org-crypt-disable-auto-save'." (heading-was-invisible-p (save-excursion (outline-end-of-heading) - (outline-invisible-p)))) + (org-invisible-p)))) (org-end-of-meta-data) (when (looking-at "-----BEGIN PGP MESSAGE-----") (org-crypt-check-auto-save) diff --git a/lisp/org-list.el b/lisp/org-list.el index a84452d30b..471ad59cf1 100644 --- a/lisp/org-list.el +++ b/lisp/org-list.el @@ -154,8 +154,8 @@ (declare-function org-timer-item "org-timer" (&optional arg)) (declare-function org-trim "org" (s &optional keep-lead)) (declare-function org-uniquify "org" (list)) +(declare-function org-invisible-p "org" (&optional pos)) (declare-function outline-flag-region "outline" (from to flag)) -(declare-function outline-invisible-p "outline" (&optional pos)) (declare-function outline-next-heading "outline" ()) (declare-function outline-previous-heading "outline" ()) @@ -2257,7 +2257,7 @@ item is invisible." (unless (or (not itemp) (save-excursion (goto-char itemp) - (outline-invisible-p))) + (org-invisible-p))) (if (save-excursion (goto-char itemp) (org-at-item-timer-p)) diff --git a/lisp/org.el b/lisp/org.el index e2e56f2284..722717f4ec 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -6880,6 +6880,11 @@ Use `\\[org-edit-special]' to edit table.el tables")) (defvar org-called-with-limited-levels nil "Non-nil when `org-with-limited-levels' is currently active.") +(defun org-invisible-p (&optional pos) + "Non-nil if the character after POS is invisible. +If POS is nil, use `point' instead." + (get-char-property (or pos (point)) 'invisible)) + (defun org-cycle-internal-local () "Do the local cycling action." (let ((goal-column 0) eoh eol eos has-children children-skipped struct) @@ -6923,7 +6928,7 @@ Use `\\[org-edit-special]' to edit table.el tables")) (save-excursion (goto-char eos) (outline-next-heading) - (when (outline-invisible-p) (org-flag-heading nil)))) + (when (org-invisible-p) (org-flag-heading nil)))) ((and (or (>= eol eos) (not (string-match "\\S-" (buffer-substring eol eos)))) (or has-children @@ -6957,7 +6962,7 @@ Use `\\[org-edit-special]' to edit table.el tables")) (save-excursion (goto-char eos) (outline-next-heading) - (when (outline-invisible-p) (org-flag-heading nil))) + (when (org-invisible-p) (org-flag-heading nil))) (setq org-cycle-subtree-status 'children) (unless (org-before-first-heading-p) (run-hook-with-args 'org-cycle-hook 'children))) @@ -7131,9 +7136,9 @@ This function is the default value of the hook `org-cycle-hook'." ;; Properly fold already folded siblings (goto-char (point-min)) (while (re-search-forward re nil t) - (when (and (not (outline-invisible-p)) + (when (and (not (org-invisible-p)) (save-excursion - (goto-char (point-at-eol)) (outline-invisible-p))) + (goto-char (point-at-eol)) (org-invisible-p))) (outline-hide-entry)))) (org-cycle-show-empty-lines 'overview) (org-cycle-hide-drawers 'overview))))) @@ -7486,7 +7491,7 @@ With a prefix argument, use the alternative interface: e.g., if (progn (org-mark-ring-push org-goto-start-pos) (goto-char selected-point) - (when (or (outline-invisible-p) (org-invisible-p2)) + (when (or (org-invisible-p) (org-invisible-p2)) (org-show-context 'org-goto))) (message "Quit")))) @@ -7527,7 +7532,7 @@ or nil." (if (and (boundp 'org-goto-start-pos) (integer-or-marker-p org-goto-start-pos)) (progn (goto-char org-goto-start-pos) - (when (outline-invisible-p) + (when (org-invisible-p) (org-show-set-visibility 'lineage))) (goto-char (point-min))) (let (org-special-ctrl-a/e) (org-beginning-of-line)) @@ -8384,7 +8389,7 @@ case." (setq beg (point))) (save-match-data (save-excursion (outline-end-of-heading) - (setq folded (outline-invisible-p))) + (setq folded (org-invisible-p))) (progn (org-end-of-subtree nil t) (unless (eobp) (backward-char)))) (outline-next-heading) @@ -8480,7 +8485,7 @@ useful if the caller implements cut-and-paste as copy-then-paste-then-cut." (if nosubtrees (outline-next-heading) (save-excursion (outline-end-of-heading) - (setq folded (outline-invisible-p))) + (setq folded (org-invisible-p))) (ignore-errors (org-forward-heading-same-level (1- n) t)) (org-end-of-subtree t t))) ;; Include the end of an inlinetask @@ -8533,7 +8538,7 @@ When REMOVE is non-nil, remove the subtree from the clipboard." (substitute-command-keys "The kill is not a (set of) tree(s) - please use \\[yank] to yank anyway"))) (org-with-limited-levels - (let* ((visp (not (outline-invisible-p))) + (let* ((visp (not (org-invisible-p))) (txt tree) (^re_ "\\(\\*+\\)[ \t]*") (old-level (if (string-match org-outline-regexp-bol txt) @@ -8590,7 +8595,7 @@ When REMOVE is non-nil, remove the subtree from the clipboard." (goto-char beg) (skip-chars-forward " \t\n\r") (setq beg (point)) - (when (and (outline-invisible-p) visp) + (when (and (org-invisible-p) visp) (save-excursion (outline-show-heading))) ;; Shift if necessary (unless (= shift 0) @@ -10438,7 +10443,7 @@ If the link is in hidden text, expose it." (if (funcall srch-fun org-any-link-re nil t) (progn (goto-char (match-beginning 0)) - (when (outline-invisible-p) (org-show-context))) + (when (org-invisible-p) (org-show-context))) (goto-char pos) (setq org-link-search-failed t) (message "No further link found")))) @@ -11141,7 +11146,7 @@ or to another Org file, automatically push the old position onto the ring." (setq m (car p)) (pop-to-buffer-same-window (marker-buffer m)) (goto-char m) - (when (or (outline-invisible-p) (org-invisible-p2)) (org-show-context 'mark-goto)))) + (when (or (org-invisible-p) (org-invisible-p2)) (org-show-context 'mark-goto)))) (defun org-add-angle-brackets (s) (unless (equal (substring s 0 1) "<") (setq s (concat "<" s))) @@ -12392,7 +12397,7 @@ When called through ELisp, arg is also interpreted in the following way: (org-map-entries `(org-todo ,arg) org-loop-over-headlines-in-active-region - cl (when (outline-invisible-p) (org-end-of-subtree nil t)))) + cl (when (org-invisible-p) (org-end-of-subtree nil t)))) (when (equal arg '(16)) (setq arg 'nextset)) (when (equal arg -1) (org-cancel-repeater) (setq arg nil)) (let ((org-blocker-hook org-blocker-hook) @@ -13320,7 +13325,7 @@ can either be an Org date like \"2011-07-24\" or a delta like \"+2d\"." (if (eq org-loop-over-headlines-in-active-region 'start-level) 'region-start-level 'region) - (lambda () (when (outline-invisible-p) (org-end-of-subtree nil t)))) + (lambda () (when (org-invisible-p) (org-end-of-subtree nil t)))) (org--deadline-or-schedule arg 'deadline time))) (defun org-schedule (arg &optional time) @@ -13337,7 +13342,7 @@ either be an Org date like \"2011-07-24\" or a delta like \"+2d\"." (if (eq org-loop-over-headlines-in-active-region 'start-level) 'region-start-level 'region) - (lambda () (when (outline-invisible-p) (org-end-of-subtree nil t)))) + (lambda () (when (org-invisible-p) (org-end-of-subtree nil t)))) (org--deadline-or-schedule arg 'scheduled time))) (defun org-get-scheduled-time (pom &optional inherit) @@ -14884,7 +14889,7 @@ When JUST-ALIGN is non-nil, only align tags." #'org-set-tags org-loop-over-headlines-in-active-region cl - '(when (outline-invisible-p) (org-end-of-subtree nil t)))) + '(when (org-invisible-p) (org-end-of-subtree nil t)))) (let ((org-setting-tags t)) (if arg (save-excursion @@ -23847,16 +23852,18 @@ interactive command with similar behavior." "Check if point is at a character currently not visible. This version does not only check the character property, but also `visible-mode'." - ;; Early versions of noutline don't have `outline-invisible-p'. (unless (bound-and-true-p visible-mode) - (outline-invisible-p))) + (org-invisible-p))) (defun org-invisible-p2 () - "Check if point is at a character currently not visible." + "Check if point is at a character currently not visible. + +If the point is at EOL (and not at the beginning of a buffer too), +move it back by one char before doing this check." (save-excursion - (when (and (eolp) (not (bobp))) (backward-char 1)) - ;; Early versions of noutline don't have `outline-invisible-p'. - (outline-invisible-p))) + (when (and (eolp) (not (bobp))) + (backward-char 1)) + (org-invisible-p))) (defun org-back-to-heading (&optional invisible-ok) "Call `outline-back-to-heading', but provide a better error message." @@ -24126,7 +24133,7 @@ non-nil it will also look at invisible ones." (cond ((< l level) (setq count 0)) ((and (= l level) (or invisible-ok - (not (outline-invisible-p + (not (org-invisible-p (line-beginning-position))))) (cl-decf count) (when (= l level) (setq result (point))))))) @@ -24262,7 +24269,7 @@ item, etc. It also provides some special moves for convenience: ;; With no contents, just skip element. ((not contents-begin) (goto-char end)) ;; If contents are invisible, skip the element altogether. - ((outline-invisible-p (line-end-position)) + ((org-invisible-p (line-end-position)) (cl-case type (headline (org-with-limited-levels (outline-next-visible-heading 1))) @@ -24349,7 +24356,7 @@ convenience: (org-backward-paragraph)) (t (goto-char (or post-affiliated begin)))) ;; Ensure we never leave point invisible. - (when (outline-invisible-p (point)) (beginning-of-visual-line)))) + (when (org-invisible-p (point)) (beginning-of-visual-line)))) (defun org-forward-element () "Move forward by one element. @@ -24426,7 +24433,7 @@ Move to the previous element at the same level, when possible." (forward-char)) ((memq (org-element-type element) org-element-greater-elements) ;; If contents are hidden, first disclose them. - (when (outline-invisible-p (line-end-position)) (org-cycle)) + (when (org-invisible-p (line-end-position)) (org-cycle)) (goto-char (or (org-element-property :contents-begin element) (user-error "No content for this element")))) (t (user-error "No inner element"))))) @@ -24907,15 +24914,15 @@ ELEMENT is the element at point." (defun org-bookmark-jump-unhide () "Unhide the current position, to show the bookmark location." (and (derived-mode-p 'org-mode) - (or (outline-invisible-p) + (or (org-invisible-p) (save-excursion (goto-char (max (point-min) (1- (point)))) - (outline-invisible-p))) + (org-invisible-p))) (org-show-context 'bookmark-jump))) (defun org-mark-jump-unhide () "Make the point visible with `org-show-context' after jumping to the mark." (when (and (derived-mode-p 'org-mode) - (outline-invisible-p)) + (org-invisible-p)) (org-show-context 'mark-goto))) (eval-after-load "simple" -- 2.13.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: Backward incompatible outline-invisible-p change in emacs master for Org 2017-06-14 14:56 ` Backward incompatible outline-invisible-p change in emacs master for Org (Was: 6 failing tests on master branch) Kaushal Modi 2017-06-14 16:03 ` Kaushal Modi @ 2017-06-14 17:02 ` Bastien Guerry 2017-06-14 17:18 ` Kaushal Modi 1 sibling, 1 reply; 19+ messages in thread From: Bastien Guerry @ 2017-06-14 17:02 UTC (permalink / raw) To: Kaushal Modi; +Cc: Paul Rankin, emacs-org list, Nicolas Goaziou Hi Kaushal, thanks for putting me in the loop. Kaushal Modi <kaushal.modi@gmail.com> writes: > I also find an org-invisible-p2 function, but it's not clear why it > does a (backward-char 1) before doing invisibility check.. and that > too eventually relies on outline-visible-p. (Turns out this function > has a long history: http://orgmode.org/cgit.cgi/org-mode.git/commit/? > id=93a4128a6fe47b4e05c0a7cb3ec14878b41d6000 ) The question is: why this patch in the first place? Paul authored it and I committed it, so I should know--but I don't. Maybe Paul know or you know better? > Here's a patch for the above proposed solution. This looks fine, but I'd rather revert the faulty Emacs commit if it is not necessary. Until Paul enlighten us, I'll have a deeper look. Thanks, -- Bastien ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Backward incompatible outline-invisible-p change in emacs master for Org 2017-06-14 17:02 ` Backward incompatible outline-invisible-p change in emacs master for Org Bastien Guerry @ 2017-06-14 17:18 ` Kaushal Modi 2017-06-14 20:28 ` Paul Rankin 0 siblings, 1 reply; 19+ messages in thread From: Kaushal Modi @ 2017-06-14 17:18 UTC (permalink / raw) To: Bastien Guerry; +Cc: Paul Rankin, emacs-org list, Nicolas Goaziou [-- Attachment #1: Type: text/plain, Size: 1774 bytes --] On Wed, Jun 14, 2017 at 1:02 PM Bastien Guerry <bzg@gnu.org> wrote: > The question is: why this patch in the first place? Paul authored it > and I committed it, so I should know--but I don't. Maybe Paul know or > you know better? > I later found the reason for that commit here: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24073 Here is the emacs -Q recipe that Paul posted on that debbugs report: 1. emacs -Q 2. insert ";;; heading" 3. M-: (outline-on-heading-p) => t 4. C-a 5. M-: (put-text-property (point) (1+ (point)) 'invisible 'foo) 6. M-; (outline-on-heading-p) => nil Expected results: (outline-on-heading-p) => t Actual results: (outline-on-heading-p) => nil This shows the relation between outline-on-heading-p and outline-invisible-p: (defun outline-on-heading-p (&optional invisible-ok) "Return t if point is on a (visible) heading line. If INVISIBLE-OK is non-nil, an invisible heading line is ok too." (save-excursion (beginning-of-line) (and (bolp) (or invisible-ok (not (outline-invisible-p))) (looking-at outline-regexp)))) Basically the expectation is that a outline heading be not marked as "invisible" by any 'foo invisible property. Outline headings should be marked invisible by only 'outline invisible property. Also as the function is prefixed with "outline-", that kind of makes sense. On the other hand, in org, we need a function that returns non-nil for *any* invisible property. So that commit breaks org's expectation. This looks fine, Thanks. > but I'd rather revert the faulty Emacs commit if > it is not necessary. > > Until Paul enlighten us, I'll have a deeper look. > After reviewing debbugs 24073, the commit looks correct for emacs master and outline package. WDYT? -- Kaushal Modi [-- Attachment #2: Type: text/html, Size: 3535 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Backward incompatible outline-invisible-p change in emacs master for Org 2017-06-14 17:18 ` Kaushal Modi @ 2017-06-14 20:28 ` Paul Rankin 2017-06-15 6:19 ` Bastien Guerry 0 siblings, 1 reply; 19+ messages in thread From: Paul Rankin @ 2017-06-14 20:28 UTC (permalink / raw) To: Kaushal Modi; +Cc: Bastien Guerry, emacs-org list, Nicolas Goaziou On Thu, 15 Jun 2017, at 03:18 AM, Kaushal Modi wrote: > On Wed, Jun 14, 2017 at 1:02 PM Bastien Guerry <bzg@gnu.org> wrote: > > > The question is: why this patch in the first place? Paul authored it > > and I committed it, so I should know--but I don't. Maybe Paul know or > > you know better? > > > > I later found the reason for that commit here: > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24073 > > Here is the emacs -Q recipe that Paul posted on that debbugs report: > > 1. emacs -Q > 2. insert ";;; heading" > 3. M-: (outline-on-heading-p) > => t > 4. C-a > 5. M-: (put-text-property (point) (1+ (point)) 'invisible 'foo) > 6. M-; (outline-on-heading-p) > => nil > > Expected results: > > (outline-on-heading-p) > => t > > Actual results: > > (outline-on-heading-p) > => nil > > This shows the relation between outline-on-heading-p and > outline-invisible-p: > > (defun outline-on-heading-p (&optional invisible-ok) > "Return t if point is on a (visible) heading line. > If INVISIBLE-OK is non-nil, an invisible heading line is ok too." > (save-excursion > (beginning-of-line) > (and (bolp) (or invisible-ok (not (outline-invisible-p))) > (looking-at outline-regexp)))) > > Basically the expectation is that a outline heading be not marked as > "invisible" by any 'foo invisible property. Outline headings should be > marked invisible by only 'outline invisible property. > > Also as the function is prefixed with "outline-", that kind of makes sense. > > On the other hand, in org, we need a function that returns non-nil for > *any* invisible property. So that commit breaks org's expectation. > > This looks fine, > > > Thanks. > > > > but I'd rather revert the faulty Emacs commit if > > it is not necessary. > > > > Until Paul enlighten us, I'll have a deeper look. > > > > After reviewing debbugs 24073, the commit looks correct for emacs master > and outline package. WDYT? Glad the mystery looks like it's solved. Btw I looped in the orgmode list back here http://lists.gnu.org/archive/html/emacs-orgmode/2016-09/msg00029.html Thanks. -- www.paulwrankin.com ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Backward incompatible outline-invisible-p change in emacs master for Org 2017-06-14 20:28 ` Paul Rankin @ 2017-06-15 6:19 ` Bastien Guerry 2017-06-15 12:16 ` Kaushal Modi 0 siblings, 1 reply; 19+ messages in thread From: Bastien Guerry @ 2017-06-15 6:19 UTC (permalink / raw) To: Paul Rankin; +Cc: emacs-org list, Nicolas Goaziou, Kaushal Modi Hi Paul, thanks for the follow-up -- Kaushal, thanks for the fix, please go ahead and commit it. Best, -- Bastien ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Backward incompatible outline-invisible-p change in emacs master for Org 2017-06-15 6:19 ` Bastien Guerry @ 2017-06-15 12:16 ` Kaushal Modi 2017-06-15 12:40 ` Bastien Guerry 0 siblings, 1 reply; 19+ messages in thread From: Kaushal Modi @ 2017-06-15 12:16 UTC (permalink / raw) To: Bastien Guerry, Nicolas Goaziou; +Cc: Paul Rankin, emacs-org list [-- Attachment #1: Type: text/plain, Size: 676 bytes --] On Thu, Jun 15, 2017, 2:19 AM Bastien Guerry <bzg@gnu.org> wrote: > Kaushal, thanks for the fix, please go > ahead and commit it. > Before I commit this to master, I was thinking that this patch looks safe enough for the maint branch. If there are no objections, I can commit this to maint and merge to master. The patch adds a new function org-invisible-p whose definition is the same as outline-invisible-p from emacs 25.2 and refactors all instances of outline-invisible-p with org-invisible-p. So the change will be transparent for people using emacs stable releases. But as a plus, people using emacs master + org maint will get this bug fix too. > -- Kaushal Modi [-- Attachment #2: Type: text/html, Size: 1230 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Backward incompatible outline-invisible-p change in emacs master for Org 2017-06-15 12:16 ` Kaushal Modi @ 2017-06-15 12:40 ` Bastien Guerry 2017-06-15 13:08 ` Kaushal Modi 0 siblings, 1 reply; 19+ messages in thread From: Bastien Guerry @ 2017-06-15 12:40 UTC (permalink / raw) To: Kaushal Modi; +Cc: Paul Rankin, emacs-org list, Nicolas Goaziou Kaushal Modi <kaushal.modi@gmail.com> writes: > Before I commit this to master, I was thinking that this patch looks > safe enough for the maint branch. If there are no objections, I can > commit this to maint and merge to master. Sure, please go ahead. Thanks! -- Bastien ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Backward incompatible outline-invisible-p change in emacs master for Org 2017-06-15 12:40 ` Bastien Guerry @ 2017-06-15 13:08 ` Kaushal Modi 0 siblings, 0 replies; 19+ messages in thread From: Kaushal Modi @ 2017-06-15 13:08 UTC (permalink / raw) To: Bastien Guerry; +Cc: Paul Rankin, emacs-org list, Nicolas Goaziou [-- Attachment #1: Type: text/plain, Size: 371 bytes --] On Thu, Jun 15, 2017 at 8:40 AM Bastien Guerry <bzg@gnu.org> wrote: > Kaushal Modi <kaushal.modi@gmail.com> writes: > > > Before I commit this to master, I was thinking that this patch looks > > safe enough for the maint branch. If there are no objections, I can > > commit this to maint and merge to master. > > Sure, please go ahead. Thanks. Done. -- Kaushal Modi [-- Attachment #2: Type: text/html, Size: 804 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2017-06-15 13:09 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-06-08 20:33 6 failing tests on master branch Kaushal Modi 2017-06-08 21:10 ` Nicolas Goaziou 2017-06-09 14:26 ` Kaushal Modi 2017-06-09 15:14 ` Kaushal Modi 2017-06-09 16:02 ` Michael Albinus 2017-06-09 16:10 ` Kaushal Modi 2017-06-09 16:27 ` Nicolas Goaziou 2017-06-09 19:55 ` Kaushal Modi 2017-06-09 20:05 ` Nicolas Goaziou 2017-06-09 20:21 ` Kaushal Modi 2017-06-14 14:56 ` Backward incompatible outline-invisible-p change in emacs master for Org (Was: 6 failing tests on master branch) Kaushal Modi 2017-06-14 16:03 ` Kaushal Modi 2017-06-14 17:02 ` Backward incompatible outline-invisible-p change in emacs master for Org Bastien Guerry 2017-06-14 17:18 ` Kaushal Modi 2017-06-14 20:28 ` Paul Rankin 2017-06-15 6:19 ` Bastien Guerry 2017-06-15 12:16 ` Kaushal Modi 2017-06-15 12:40 ` Bastien Guerry 2017-06-15 13:08 ` Kaushal Modi
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).