emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* 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).