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

ctrl-a/ctrl-x do not reliably modify dates #796

Closed KnBrckr closed 2 months ago

KnBrckr commented 2 months ago

Describe the bug

Depending on where a date is in a file, it may or may not update correctly with ctrl-a/ctrl-x (timestamp_up/timestamp_down).

When the operation fails, the number field is observed to be updated in the opposite direction from that intended, and the update is not recognized as an update to a date field.

Steps to reproduce

Using this test file:

* TODO testing <2024-08-17 Sat> 
* stuff <2024-01-17 Wed> 
  <2024-02-17 Sat> 
  <2024-03-17 Sun> 

Placing the cursor the day portion of each line and pressing ctrl-a results in the following behavior for each line:

  1. Incorrectly changes to <2024-08-16 Sat> and day of week does not change
  2. Incorrectly changes to <2024-01-16 Wed> and day of week does not change
  3. Correctly changes to <2024-02-18 Sun>
  4. Incorrectly changes to <2024-03-16 Sun> and day of week does not change

The decrement action is likely a result of nvim default behavior that interprets the value as a negative number (-17), thus increasing it to -16.

Expected behavior

ctrl-a and ctrl-x should recognize the date on each line and increment/decrement accordingly.

Emacs functionality

No response

Minimal init.lua

vim.opt.rtp:prepend(vim.env.REAL_HOME .. "/.local/share/nvim/lazy/orgmode")
require("orgmode").setup {
                        org_agenda_files = { '~/test-orgfiles/**/*', },
                        org_default_notes_file = '~/test-orgfiles/refile.org',
}

Screenshots and recordings

No response

OS / Distro

OSX 14.6.1

Neovim version/commit

NVIM v0.10.1 Build type: Release LuaJIT 2.1.1723675123

Additional context

Tested using commit adf277c

Starting from OrgMappings:_adjust_date_part(), and given that the cursor is on line 1, col 26 when ctrl-a is pressed in the sample file mentioned above, The date object referenced by date_on_cursor contains range.end_col = 29 and range.start_col=14. Considering the line of the sample file:

1234567890123456789012345678901
* TODO testing <2024-08-17 Sat>
             \-start_col     \-end_col

The values are 2 less than they should be to correctly reference this line of text. When increment/decrement work, the start/end_col values reference the locations of the date field delimiters on the line. (I haven't explored why the 4th line doesn't work but it may related to the TODO at the end of OrgMappings:_get_date_under_cursor()

I drilled down and found that in Headline:get_non_plan_dates() the value of headline_text that is used to locate the date does not include the leading * from the buffer. This would explain the off-by-two error in locating the date in the original line.

I haven't sorted out what the functions are doing to set the value of headline_text (I'm still learning Lua) but this is getting close to where the problem is. I think the fix is to ensure that the headline text represents the full line of text, but I'm both unclear how to do that, and unclear what other side effects that may have on callers that expect this section of code to be working based on tree sitter objects vs. lines of text. Given my lack of understanding, it's also possible that OrgMappings:_adjust_date_part() should be using tree sitter objects where now it appears to be using text lines.

Inspecting the first line of the example file, tree-sitter is not identifying the date on this line as a timestamp. If I move the date to its own line, inspecting the tree includes a timestamp node. Part of the problem?

KnBrckr commented 2 months ago

Why does OrgMappings:_get_date_under_cursor() go to trouble of getting the closest heading and then parsing multiple lines of text? Wouldn't it be simpler to look for enclosing <> or [] to isolate the date at the cursor position? Maybe this strategy would also address the TODO noted where it returns?

KnBrckr commented 2 months ago

Why does OrgMappings:_get_date_under_cursor() go to trouble of getting the closest heading and then parsing multiple lines of text? Wouldn't it be simpler to look for enclosing <> or [] to isolate the date at the cursor position? Maybe this strategy would also address the TODO noted where it returns?

Commit 1f103a9b added an else clause to _get_date_under_cursor() that works in all cases for this particular use case. With a slight modification it seems it could also select the correct date on a line when multiple dates appear on the line using the vim.tbl_filter() addressing the TODO.

What I'm missing is why this function should be considering other lines through the use of get_closest_headline_or_nil(). Given the filter that exists to exclude dates that do not include the cursor position, the extra work to find headlines, etc. seems unnecessary.

The following is working in my test-cases, including when multiple dates appear on a single line. I don't know enough about how this function is used to understand what impact this may have on other callers. The notable area of concern being if other callers expect the date object offsets to be based on a tree-sitter object or based on the line in the buffer.

function OrgMappings:_get_date_under_cursor(col_offset)
  col_offset = col_offset or 0
  local col = vim.fn.col('.') + col_offset
  local line = vim.fn.line('.') or 0
  local dates = {}

  dates = vim.tbl_filter(function(date)
    return date.range:is_in_range(line, col)
  end, Date.parse_all_from_line(vim.fn.getline('.'), line))

  if #dates == 0 then
    return nil
  end

  return dates[1]
end
kristijanhusak commented 2 months ago

Thanks for debugging it, it was very helpful to figure out what's the issue. I pushed a fix in d0baf31181435c2378dc2c19a57dcebcc36d9464 that fixes the date ranges in the headline, so now everything should work as expected.