nvim-orgmode / orgmode

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

org_timestamp_{up,down} with count behaves unexpectedly on minutes #396

Open Ram-Z opened 2 years ago

Ram-Z commented 2 years ago

Describe the bug

When using org_timestamp_{up,down} with a count on minutes, the value is changed by count times org_time_stamp_rounding_minutes rather than count minutes.

Also there's inconsistent spelling of timestamp and time_stamp in the options.

Steps to reproduce

  1. set org_time_stamp_rounding_minutes = 5
  2. put cursor on minutes part of datetime
  3. do 15<C-A>

Expected behavior

15<C-A> increases 15 minutes regardless of the value of org_time_stamp_rounding_minutes.

Emacs functionality

No response

Minimal init.lua

Reproducible on default settings.

Screenshots and recordings

No response

OS / Distro

Arch

Neovim version/commit

NVIM v0.7.2

Additional context

No response

jgollenz commented 2 years ago

I am not sure if I understood your issue. The behavior you describe is the one I would expect. You are running <C-a> fifteen times, each one increasing the time by 5 minutes.

The name org_time_stamp_rounding_minutes comes from Emacs, I think org_timestamp_up is exclusive to this port. So it should probably be org_time_stamp_[up|down].

Ram-Z commented 2 years ago

Yeah, I understand the current behaviour is explained by #<C-A> simply applying <C-A> # times. I'd argue that that is unexpected though. When you give a count, you probably know exactly how many minutes you want to increment/decrement by, you should not be needing to do mental acrobatics to figure out how many multiples of org_time_stamp_rounding_minutes you want.

Your comment prompted me to look up what emacs actually does (I've not used it before, so usually don't compare). And emacs behaves very differently from the current nvim-orgmode.

From org-time-stamp-rounding-minutes:

Number of minutes to round time stamps to.

This applies to the timestamp, not the step size, f.ex [2022-08-18 Thu 08:2|2] <C-A> -> [2022-08-18 Thu 08:2|5] <C-A> -> [2022-08-18 Thu 08:3|0], i.e. the first <C-A> rounds it to the nearest org_time_stamp_rounding_minutes then starts incrementing by org_time_stamp_rounding_minutes.

Furthermore:

When this is larger than 1, you can still force an exact time stamp [...] by using a prefix arg to `S-up/down' to specify the exact number of minutes to shift.

Which is exactly the behaviour I'm requesting here.

I've verified this behaviour in spacemacs.

kristijanhusak commented 2 years ago

This applies to the timestamp, not the step size, f.ex [2022-08-18 Thu 08:2|2] -> [2022-08-18 Thu 08:2|5] -> [2022-08-18 Thu 08:3|0], i.e. the first rounds it to the nearest org_time_stamp_rounding_minutes then starts incrementing by org_time_stamp_rounding_minutes.

You are right about this. It's not rounding it up, it just jumps by that number of minutes.

Regarding this:

When this is larger than 1, you can still force an exact time stamp [...] by using a prefix arg to `S-up/down' to specify the exact number of minutes to shift.

Do you know which prefix needs to be used in Emacs to achieve this? I wasn't able to find it.

Ram-Z commented 2 years ago

In spacemacs it was just 42<S-Up> in normal mode, for vanilla emacs it seems to be <C-U>42<S-Up>.

Ram-Z commented 2 years ago

Here's a quick and dirty patch to address the <count> issue. It does not address the rounding.

diff --git a/lua/orgmode/org/mappings.lua b/lua/orgmode/org/mappings.lua
index 483371b..831814f 100644
--- a/lua/orgmode/org/mappings.lua
+++ b/lua/orgmode/org/mappings.lua
@@ -180,11 +180,11 @@ function OrgMappings:timestamp_down_day()
 end

 function OrgMappings:timestamp_up()
-  return self:_adjust_date_part('+', vim.v.count1, config.mappings.org.org_timestamp_up)
+  return self:_adjust_date_part('+', vim.v.count, config.mappings.org.org_timestamp_up)
 end

 function OrgMappings:timestamp_down()
-  return self:_adjust_date_part('-', vim.v.count1, config.mappings.org.org_timestamp_down)
+  return self:_adjust_date_part('-', vim.v.count, config.mappings.org.org_timestamp_down)
 end

 function OrgMappings:_adjust_date_part(direction, amount, fallback)
@@ -192,7 +192,13 @@ function OrgMappings:_adjust_date_part(direction, amount, fallback)
   local get_adj = function(span, count)
     return string.format('%d%s', count or amount, span)
   end
-  local minute_adj = get_adj('M', tonumber(config.org_time_stamp_rounding_minutes) * amount)
+  local minute_adj
+  if amount == 0 then
+    minute_adj = get_adj('M', tonumber(config.org_time_stamp_rounding_minutes))
+    amount = 1  -- default to 1 for any other part
+  else
+    minute_adj = get_adj('M', amount)
+  end
   local do_replacement = function(date)
     local col = vim.fn.col('.')
     local char = vim.fn.getline('.'):sub(col, col)
kristijanhusak commented 2 years ago

Here's a quick and dirty patch to address the <count> issue. It does not address the rounding.

diff --git a/lua/orgmode/org/mappings.lua b/lua/orgmode/org/mappings.lua
index 483371b..831814f 100644
--- a/lua/orgmode/org/mappings.lua
+++ b/lua/orgmode/org/mappings.lua
@@ -180,11 +180,11 @@ function OrgMappings:timestamp_down_day()
 end

 function OrgMappings:timestamp_up()
-  return self:_adjust_date_part('+', vim.v.count1, config.mappings.org.org_timestamp_up)
+  return self:_adjust_date_part('+', vim.v.count, config.mappings.org.org_timestamp_up)
 end

 function OrgMappings:timestamp_down()
-  return self:_adjust_date_part('-', vim.v.count1, config.mappings.org.org_timestamp_down)
+  return self:_adjust_date_part('-', vim.v.count, config.mappings.org.org_timestamp_down)
 end

 function OrgMappings:_adjust_date_part(direction, amount, fallback)
@@ -192,7 +192,13 @@ function OrgMappings:_adjust_date_part(direction, amount, fallback)
   local get_adj = function(span, count)
     return string.format('%d%s', count or amount, span)
   end
-  local minute_adj = get_adj('M', tonumber(config.org_time_stamp_rounding_minutes) * amount)
+  local minute_adj
+  if amount == 0 then
+    minute_adj = get_adj('M', tonumber(config.org_time_stamp_rounding_minutes))
+    amount = 1  -- default to 1 for any other part
+  else
+    minute_adj = get_adj('M', amount)
+  end
   local do_replacement = function(date)
     local col = vim.fn.col('.')
     local char = vim.fn.getline('.'):sub(col, col)

Something like this will work, but I would love to have a separate mapping for this, the same as emacs has a prefix.

Normal rounding minute jumps would go like now (with the fix for the rounding) And doing that count thing can be done with something like <leader><c-a> and <leader><c-x>.

Thoughts?

jgollenz commented 2 years ago

Where would the count go in <leader><c-a>?

kristijanhusak commented 2 years ago

Where would the count go in <leader><c-a>?

Same as for any vim mapping, 42<leader><c-a>

Ram-Z commented 2 years ago

I would love to have a separate mapping for this, the same as emacs has a prefix.

emacs has a prefix because emacs does not have normal mode, typing the count without the prefix would result in those numbers simply being inserted, i.e. think of the <C-U> prefix in emacs as part of the count. Which is why <C-U> is not needed in normal mode in spacemacs too. nvim just happens to use <C-A> instead of <S-Up>.