nvim-orgmode / orgmode

Orgmode clone written in Lua for Neovim 0.9+.
https://nvim-orgmode.github.io/
MIT License
3.03k stars 134 forks source link

Refiling sections on the same file gives unexpected results #633

Closed lyz-code closed 9 months ago

lyz-code commented 11 months ago

Describe the bug

When refiling a section to another in the same file that is below the result is not the expected

Steps to reproduce

In the file test.org with the contents:

* Heading 1
** Subheading 1 
** Subheading 2
* Heading 2     
**** SubSubheading 2.1    

Place the cursor over Subheading 1 and try to refile to test.org:SubSubheading 2.1

Expected behavior

The section is well refined, so the result file is:

* Heading 1
** Subheading 2
* Heading 2     
**** SubSubheading 2.1   
***** Subheading 1 

Right now the output is:

* Heading 1
***** Subheading 1
** Subheading 2
* Heading 2     
**** SubSubheading 2.1    

Emacs functionality

I guess it works as expected

Minimal init.lua

vim.g.mapleader = ' '

vim.cmd([[set runtimepath=$VIMRUNTIME]])
vim.cmd([[set packpath=/tmp/nvim/site]])

local package_root = '/tmp/nvim/site/pack'
local install_path = package_root .. '/packer/start/packer.nvim'

local function load_plugins()
  require('packer').startup({
    {
      'wbthomason/packer.nvim',
      { 'nvim-treesitter/nvim-treesitter' },
      { 'kristijanhusak/orgmode.nvim', branch = 'master' },
    },
    config = {
      package_root = package_root,
      compile_path = install_path .. '/plugin/packer_compiled.lua',
    },
  })
end

_G.load_config = function()
  require('orgmode').setup_ts_grammar()
  require('nvim-treesitter.configs').setup({
    highlight = {
      enable = true,
      additional_vim_regex_highlighting = { 'org' },
    },
  })

  vim.cmd([[packadd nvim-treesitter]])
  vim.cmd([[runtime plugin/nvim-treesitter.lua]])
  vim.cmd([[TSUpdateSync org]])

  -- Close packer after install
  if vim.bo.filetype == 'packer' then
    vim.api.nvim_win_close(0, true)
  end

  require('orgmode').setup({
    org_agenda_files = {
      './*'
    }
  }
  )

  -- Reload current file if it's org file to reload tree-sitter
  if vim.bo.filetype == 'org' then
    vim.cmd([[edit!]])
  end
end

if vim.fn.isdirectory(install_path) == 0 then
  vim.fn.system({ 'git', 'clone', 'https://github.com/wbthomason/packer.nvim', install_path })
  load_plugins()
  require('packer').sync()
  vim.cmd([[autocmd User PackerCompileDone ++once lua load_config()]])
else
  load_plugins()
  load_config()
end

Screenshots and recordings

No response

OS / Distro

Debian

Neovim version/commit

0.9.1

Additional context

If instead you place the cursor over SubSubheading 2.1 and try to refile to test.org:Subheading 1 the issue is similar. The expected output is:

* Heading 1
** Subheading 1 
*** SubSubheading 2.1  
** Subheading 2
* Heading 2     

And right now it returns:

* Heading 1
** Subheading 1
** Subheading 2
* Heading 2
*** SubSubheading 2.1 
seflue commented 10 months ago

Yes, the Refiling feature has some unexpected bugs. I'm currently working on understanding the code, because I want to fix this. My current understanding is, that the correct nesting/promotion of the subheading is done, but the "move" part of the refiling is somehow skipped.

I also observed, that when I refile a subheading into a file without a target headline, the contend is moved, but the subheading is not promoted to the topmost-one, which I would actually expect. Otherwise, if I do a lot of refiling of subheadings with different indent levels into one file, I get a completly messed-up structure instead of a flat list. @kristijanhusak: This behavior is actually encoded in a unit test, so I would like to ask you, if this behaviour is intentional. If so, I would like to make it optional over a configuration variable, because for my daily use it is actually quite annoying.

kristijanhusak commented 10 months ago

that the correct nesting/promotion of the subheading is done, but the "move" part of the refiling is somehow skipped.

Moving in the same file is done here https://github.com/nvim-orgmode/orgmode/blob/master/lua/orgmode/capture/init.lua#L352. It works differently because there's no need to open same file remotely to move things around.

I also observed, that when I refile a subheading into a file without a target headline, the contend is moved, but the subheading is not promoted to the topmost-one, which I would actually expect

If I'm not mistaken, that's how Emacs orgmode works, but it's been a while. If you can double check how it behaves there that would be great.

seflue commented 10 months ago

@kristijanhusak: Yeah, I actually found this function already and was already pretty sure, that the bug is right there. I'm still investigating, how the surrounding code actually works to implement my convenience wish, but for now here is the fix for the particular problem: #637

kristijanhusak commented 9 months ago

@lyz-code can you check if the issue is fixed now?

seflue commented 9 months ago

I also observed, that when I refile a subheading into a file without a target headline, the contend is moved, but the subheading is not promoted to the topmost-one, which I would actually expect

If I'm not mistaken, that's how Emacs orgmode works, but it's been a while. If you can double check how it behaves there that would be great.

I actually exhumed and repaired my 2years old emacs config to try this out and I realized the following:

Here is an example:

(setq org-refile-targets '((org-agenda-files :maxlevel . 3))

For simplicity I would keep the possibility to directly refile to files, which actually has also value. But if no Heading is given, we should promote the headline to the first level, because it allows for a mutch cleaner and deterministic workflow. @kristijanhusak If you agree, I will implement this and open a PR. Otherwise it probably ends up in a personal customization on my fork. 😉

kristijanhusak commented 9 months ago

If you agree, I will implement this and open a PR. Otherwise it probably ends up in a personal customization on my fork.

Yeah, go for it. Thanks for investigating!

seflue commented 9 months ago

If you agree, I will implement this and open a PR. Otherwise it probably ends up in a personal customization on my fork.

Yeah, go for it. Thanks for investigating!

Done: #641

seflue commented 9 months ago

If you agree, I will implement this and open a PR. Otherwise it probably ends up in a personal customization on my fork.

Yeah, go for it. Thanks for investigating!

I ran the tests on my local machine against the current nightly: image Everything is green here. image

Is it possible, to rerun the failed tests in Github actions? It seems, that I don't have the permissions for that.

kristijanhusak commented 9 months ago

@seflue I just re-run last github action https://github.com/nvim-orgmode/orgmode/actions/runs/7372025475?pr=642 and it failed on the same thing. We'll see once we merge some of these other PRs if the issue persists.

lyz-code commented 9 months ago

Sorry for the late response I've been away on holidays ^^.

The fix works perfect for me, thank you so much @seflue <3!!

May we close the issue? or wait for the CI thing to be fixed?

kristijanhusak commented 9 months ago

Since it's fixed, I'm closing this. Thanks for reporting!