Fix subtree move (fixes #752)

[seflue]

Allows to move subtrees around smoothly (e.g. using Hydra.nvim, see #752). It also fixes the foldings after changing the subtree and preserves the folding state in the current subtree when moving it up or down.

[kristijanhusak]

I think there is a simpler and shorter solution to this. We can just jump to the target line after moving.
This patch works ok for me:

diff --git a/lua/orgmode/org/mappings.lua b/lua/orgmode/org/mappings.lua
index 41d7c9f..7272f52 100644
--- a/lua/orgmode/org/mappings.lua
+++ b/lua/orgmode/org/mappings.lua
@@ -763,6 +763,7 @@ function OrgMappings:move_subtree_up()
   end
   local range = item:get_range()
   vim.cmd(string.format(':%d,%dmove %d', range.start_line, range.end_line, prev_headline:get_range().start_line - 1))
+  vim.fn.cursor(prev_headline:get_range().start_line, 1)
 end
 
 function OrgMappings:move_subtree_down()
@@ -773,6 +774,7 @@ function OrgMappings:move_subtree_down()
   end
   local range = item:get_range()
   vim.cmd(string.format(':%d,%dmove %d', range.start_line, range.end_line, next_headline:get_range().end_line))
+  vim.fn.cursor(next_headline:get_range().end_line - 2, 1)
 end
 
 function OrgMappings:show_help(type)

I'd prefer not to touch the folds with zx because that will recalculate everything and close other stuff that user might not want to be closed.

[seflue]

Would it be ok for you, if I add a configuration variable, which is false by default, but if set to true, will fix the foldings? Because I find myself pressing zx all the time and it would make my workflow much more convenient, if Orgmode would do that for me.

[kristijanhusak]

We can fire an event similar to what we do for promote/demote (example:

orgmode/lua/orgmode/org/mappings.lua

Line 502 in e91b94c

EventManager.dispatch(events.HeadlineDemoted:new(self.files:get_closest_headline(), old_level))

), and then you can listen for that in your config to do whatever you want.
You can call the events HeadlineMovedUp and HeadlineMovedDown

[seflue]

HeadlineDemoted

Two questions:

First, should we distinguish between up and down? Is there a difference or is just "HeadlineMoved" sufficient? My feeling is, that one event is enough, because it does not make much of a difference from perspective of a user. At least I currently cannot imagine a use case, where it makes a difference.

Second, is there some example code, how the events can be used from the config? A fast search over the help file didn't get any results.

For now I will just remove the fold fixes and would like to get the PR merged as it is and would open another to add the events.

[seflue]

I think there is a simpler and shorter solution to this. We can just jump to the target line after moving. This patch works ok for me:

diff --git a/lua/orgmode/org/mappings.lua b/lua/orgmode/org/mappings.lua
index 41d7c9f..7272f52 100644
--- a/lua/orgmode/org/mappings.lua
+++ b/lua/orgmode/org/mappings.lua
@@ -763,6 +763,7 @@ function OrgMappings:move_subtree_up()
   end
   local range = item:get_range()
   vim.cmd(string.format(':%d,%dmove %d', range.start_line, range.end_line, prev_headline:get_range().start_line - 1))
+  vim.fn.cursor(prev_headline:get_range().start_line, 1)
 end
 
 function OrgMappings:move_subtree_down()
@@ -773,6 +774,7 @@ function OrgMappings:move_subtree_down()
   end
   local range = item:get_range()
   vim.cmd(string.format(':%d,%dmove %d', range.start_line, range.end_line, next_headline:get_range().end_line))
+  vim.fn.cursor(next_headline:get_range().end_line - 2, 1)
 end
 
 function OrgMappings:show_help(type)

I'd prefer not to touch the folds with zx because that will recalculate everything and close other stuff that user might not want to be closed.

vim.fn.cursor(prev_headline:get_range().start_line, 1)

Your patch does not work for me, at least not for moving the headline down. I backed everything with tests, with my code they succeed, with yours they fail. I simplified the code anyway though. Hope that works for you.

[kristijanhusak]

First, should we distinguish between up and down? Is there a difference or is just "HeadlineMoved" sufficient?

You're right, one event is enough. Just provide an argument to the event which way it went.

Second, is there some example code, how the events can be used from the config? A fast search over the help file didn't get any results.

There isn't since I wanted to keep it internal, and expose it only through the api. For now, you can do something like this (Example with TodoChanged event):

  local eventManager = require('orgmode.events')

  eventManager.listen(eventManager.event.TodoChanged, function()
    print('todo changed')
  end)

Your patch does not work for me, at least not for moving the headline down

I'll take another look. I tested with the example from the issue you reported.

[seflue]

First, should we distinguish between up and down? Is there a difference or is just "HeadlineMoved" sufficient?

You're right, one event is enough. Just provide an argument to the event which way it went.

Second, is there some example code, how the events can be used from the config? A fast search over the help file didn't get any results.

There isn't since I wanted to keep it internal, and expose it only through the api. For now, you can do something like this (Example with TodoChanged event):

  local eventManager = require('orgmode.events')
  local eventTypes = require('orgmode.events.types')

  eventManager.listen(eventTypes.TodoChanged, function()
    print('todo changed')
  end)

Thanks for the insight, will try that out - but as I said, in an upcomping PR.

Your patch does not work for me, at least not for moving the headline down

I'll take another look. I tested with the example from the issue you reported.

I mean, it's quite simple math: If you move a subtree of arbitrary length in the file down, then this arbitrary length has to be somehow included in the calculation of the new cursor position, because moving the current subtree down does basically mean, pulling the next subtree this arbitrary length up. So to calculate the new cursor position after the move from a line number before the move has to subtract the number of lines of our moving subtree.
Determining the cursor position after moving the subtree up is much simpler though, because the target line has the same line number before and after the move.

[kristijanhusak]

Ok yeah I didn't test all the cases. Looks good, thanks!