Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
KnBrckr opened this issue Aug 18, 2024 · 3 comments
Closed

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

KnBrckr opened this issue Aug 18, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@KnBrckr
Copy link

KnBrckr commented Aug 18, 2024

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 KnBrckr added the bug Something isn't working label Aug 18, 2024
@KnBrckr
Copy link
Author

KnBrckr commented Aug 18, 2024

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
Copy link
Author

KnBrckr commented Aug 18, 2024

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 1f103a9 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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants