[O] Wrong non-interactive behavior of org-insert-heading breaks org-mobile-pull

[James Harkins]

I've been working for a while to track down a problem that manifests in 
org-mobile-pull. It turns out the issue is actually in org-mode itself.

Specifically (I think this is the right conclusion), if 
org-insert-heading-respect-content is called non-interactively, and the 
heading on which it's operating is invisible, then it actually inserts 
the heading under the previous higher-level visible heading.

Example: I have a testing org file, synced up to MobileOrg, containing 
in part:

* Top level 2
** Second level 3
*** Under second 3
** Second level 4

On my phone, I added a new heading "Test node C" under the parent "Under 
second 3."

Expected result of org-mobile-pull:

* Top level 2
** Second level 3
*** Under second 3
**** Test node C
** Second level 4

Actual result of org-mobile-pull:

* Top level 2
** Second level 3
*** Under second 3
** Second level 4
** Test node C


~~ Exhaustive findings from edebug sessions follow:

The way it's supposed to work is:

1. Find the parent (in this case, by olp).
2. Go to the end of the line.
3. org-insert-heading-respect-content: This should locate the end of the 
third level subtree (which is the same as the beginning of "Second level 
4"), and then insert a new third level heading "*** " here.
4. Then org-mobile-edit demotes the new subheading and inserts the content.

But, if I do org-mobile-pull with all of the top-level headings 
collapsed, what actually happens is:

1. Find the parent (in this case, by olp).
2. Go to the end of the line.
3. org-insert-heading-respect-content: First shows the parent subtree 
(the code snip is from org-insert-heading):

     (t
      ;; somewhere in the line
          (save-excursion
        (setq previous-pos (point-at-bol))
            (end-of-line)
            (setq hide-previous (outline-invisible-p)))
      (and org-insert-heading-respect-content (org-show-subtree))

(org-show-subtree) calls (org-end-of-subtree), and this identifies the 
right point. But, showing the subtree (at least in my installation) 
shows the node's payload and any child nodes, but *it does not show the 
heading itself*.

Then a little bit later we get:

          (when (featurep 'org-inlinetask)
        (while (and (not (eobp))
                (looking-at "\\(\\*+\\)[ \t]+")
                (>= (length (match-string 1))
                org-inlinetask-min-level))
          (org-end-of-subtree nil t)))

I'm not using inline tasks, so this falls through to (org-end-of-subtree 
nil t). This call explicitly clears the INVISIBLE-OK flag... so, the 
end-of-subtree location that it identifies is the next visible 
heading**and NOT the right place (which is hidden). This is the next 
top-level heading (because we started with all the top headings collapsed).

Then, continuing blindly on, a new top-level heading is added, then 
demoted, and the fourth level heading but I added on my phone has become 
a second-level heading.

** Note: org-end-of-subtree reads the wrong level of the "current" 
heading in this instance, I think because it does (org-back-to-heading 
invisible-ok). Since invisible-ok is false, this would go back to the 
previous heading that is exposed, which in my test is top-level.

So it seems this particular call to org-end-of-subtree assumes that the 
correct parent node is visible at that moment, but this assumption is 
not necessarily valid during org-mobile-pull.

I suppose it could be fixed by changing 'nil' to 't' but I have no idea 
if this would break something else. I'll leave that to the experts to 
decide.

hjh