feat(priority): improve priority cycling (#817)

Co-authored-by: Sebastian Flügge <sebastian.fluegge@dnv.com>

Author: seflue

lua/orgmode/api/headline.lua

@@ -113,7 +113,8 @@ function OrgHeadline:priority_up()
     local headline = org.files:get_closest_headline()
     local current_priority = headline:get_priority()
     local prio_range = config:get_priority_range()
-    local priority_state = PriorityState:new(current_priority, prio_range)
+    local start_with_default = config.org_priority_start_cycle_with_default
+    local priority_state = PriorityState:new(current_priority, prio_range, start_with_default)
     return headline:set_priority(priority_state:increase())
   end)
 end
@@ -125,7 +126,8 @@ function OrgHeadline:priority_down()
     local headline = org.files:get_closest_headline()
     local current_priority = headline:get_priority()
     local prio_range = config:get_priority_range()
-    local priority_state = PriorityState:new(current_priority, prio_range)
+    local start_with_default = config.org_priority_start_cycle_with_default
+    local priority_state = PriorityState:new(current_priority, prio_range, start_with_default)
     return headline:set_priority(priority_state:decrease())
   end)
 end

lua/orgmode/config/defaults.lua

@@ -32,6 +32,7 @@ local DefaultConfig = {
   org_priority_highest = 'A',
   org_priority_default = 'B',
   org_priority_lowest = 'C',
+  org_priority_start_cycle_with_default = true,
   org_archive_location = '%s_archive::',
   org_tags_column = -80,
   org_use_tag_inheritance = true,

lua/orgmode/config/init.lua

@@ -345,7 +345,11 @@ function Config:get_priorities()
     [self.opts.org_priority_highest] = { type = 'highest', hl_group = '@org.priority.highest' },
   }
 
-  local current_prio = PriorityState:new(self.opts.org_priority_highest, self:get_priority_range())
+  local current_prio = PriorityState:new(
+    self.opts.org_priority_highest,
+    self:get_priority_range(),
+    self.org_priority_start_cycle_with_default
+  )
   while current_prio:as_num() < current_prio:default_as_num() do
     current_prio:decrease()
     priorities[current_prio.priority] = { type = 'high', hl_group = '@org.priority.high' }

lua/orgmode/objects/priority_state.lua

@@ -4,18 +4,20 @@ local utils = require('orgmode.utils')
 ---@field high_priority string
 ---@field low_priority string
 ---@field priority string
+---@field start_with_default boolean
 ---@field default_priority string
 local PriorityState = {}
 
 ---@param priority string
 ---@param prio_range { highest: string, lowest: string, default: string }
-function PriorityState:new(priority, prio_range)
+function PriorityState:new(priority, prio_range, start_with_default)
   local o = {}
 
   o.high_priority = tostring(prio_range.highest)
   o.low_priority = tostring(prio_range.lowest)
   o.default_priority = tostring(prio_range.default)
   o.priority = tostring(priority or o.default_priority)
+  o.start_with_default = start_with_default
 
   setmetatable(o, self)
   self.__index = self
@@ -54,9 +56,12 @@ end
 
 ---@return string
 function PriorityState:increase()
-  if self.priority == self.high_priority then
-    self.priority = ''
-  elseif self.priority == '' then
+  if self.priority == '' then
+    self.priority = self.default_priority
+    if not self.start_with_default then
+      self.priority = self:_apply(-1)
+    end
+  elseif self.priority == self.high_priority then
     self.priority = self.low_priority
   else
     self.priority = self:_apply(-1)
@@ -67,9 +72,12 @@ end
 
 ---@return string
 function PriorityState:decrease()
-  if self.priority == self.low_priority then
-    self.priority = ''
-  elseif self.priority == '' then
+  if self.priority == '' then
+    self.priority = self.default_priority
+    if not self.start_with_default then
+      self.priority = self:_apply(1)
+    end
+  elseif self.priority == self.low_priority then
     self.priority = self.high_priority
   else
     self.priority = self:_apply(1)

lua/orgmode/org/mappings.lua

@@ -323,7 +323,7 @@ function OrgMappings:set_priority(direction)
   local headline = self.files:get_closest_headline()
   local current_priority = headline:get_priority()
   local prio_range = config:get_priority_range()
-  local priority_state = PriorityState:new(current_priority, prio_range)
+  local priority_state = PriorityState:new(current_priority, prio_range, config.org_priority_start_cycle_with_default)
 
   local new_priority = direction
   if direction == 'up' then

tests/plenary/api/api_spec.lua

@@ -3,6 +3,7 @@ local api = require('orgmode.api')
 local Date = require('orgmode.objects.date')
 local OrgId = require('orgmode.org.id')
 local orgmode = require('orgmode')
+local config = require('orgmode.config')
 
 describe('Api', function()
   ---@return OrgApiFile
@@ -109,7 +110,7 @@ describe('Api', function()
     assert.is.True(vim.fn.getline(5):match(':PERSONAL:HEALTH:$') ~= nil)
   end)
 
-  it('should toggle priority up and down', function()
+  it('should cycle upwards through priorities, starting with default', function()
     helpers.create_file({
       '#TITLE: First file',
       '',
@@ -125,24 +126,123 @@ describe('Api', function()
     local current_file = cur_file()
     local headline = current_file.headlines[2]
     assert.are.same('', headline.priority)
-    headline:priority_up():wait()
-    assert.are.same('C', cur_file().headlines[2].priority)
-    assert.is.True(vim.fn.getline(5):match('%[#C%]') ~= nil)
+    assert.are.same(true, config.org_priority_start_cycle_with_default)
+    assert.are.same('B', config.org_priority_default)
+
     headline:priority_up():wait()
     assert.are.same('B', cur_file().headlines[2].priority)
     assert.is.True(vim.fn.getline(5):match('%[#B%]') ~= nil)
     headline:priority_up():wait()
     assert.are.same('A', cur_file().headlines[2].priority)
     assert.is.True(vim.fn.getline(5):match('%[#A%]') ~= nil)
     headline:priority_up():wait()
-    assert.are.same('', cur_file().headlines[2].priority)
-    assert.is.True(vim.fn.getline(5):match('%[.*%]') == nil)
+    assert.are.same('C', cur_file().headlines[2].priority)
+    assert.is.True(vim.fn.getline(5):match('%[#C%]') ~= nil)
+    headline:priority_up():wait()
+    assert.are.same('B', cur_file().headlines[2].priority)
+    assert.is.True(vim.fn.getline(5):match('%[#B%]') ~= nil)
+  end)
+
+  it('should cycle downwards through priorities, starting with default', function()
+    helpers.create_file({
+      '#TITLE: First file',
+      '',
+      '* TODO Test orgmode :WORK:OFFICE:',
+      '  DEADLINE: <2021-07-21 Wed 22:02>',
+      '** TODO Second level :NESTEDTAG:',
+      '  DEADLINE: <2021-07-21 Wed 22:02>',
+      '* DONE Some task',
+      '  DEADLINE: <2021-07-21 Wed 22:02>',
+    })
+
+    assert.is.True(#api.load() > 1)
+    local current_file = cur_file()
+    local headline = current_file.headlines[2]
+    assert.are.same('', headline.priority)
+    assert.are.same(true, config.org_priority_start_cycle_with_default)
+    assert.are.same('B', config.org_priority_default)
+
+    headline:priority_down():wait()
+    assert.are.same('B', cur_file().headlines[2].priority)
+    assert.is.True(vim.fn.getline(5):match('%[#B%]') ~= nil)
+    headline:priority_down():wait()
+    assert.are.same('C', cur_file().headlines[2].priority)
+    assert.is.True(vim.fn.getline(5):match('%[#C%]') ~= nil)
     headline:priority_down():wait()
     assert.are.same('A', cur_file().headlines[2].priority)
     assert.is.True(vim.fn.getline(5):match('%[#A%]') ~= nil)
     headline:priority_down():wait()
     assert.are.same('B', cur_file().headlines[2].priority)
     assert.is.True(vim.fn.getline(5):match('%[#B%]') ~= nil)
+  end)
+
+  it('should enable priority at default + 1', function()
+    helpers.create_file({
+      '#TITLE: First file',
+      '',
+      '* TODO Test orgmode :WORK:OFFICE:',
+      '  DEADLINE: <2021-07-21 Wed 22:02>',
+      '** TODO Second level :NESTEDTAG:',
+      '  DEADLINE: <2021-07-21 Wed 22:02>',
+      '* DONE Some task',
+      '  DEADLINE: <2021-07-21 Wed 22:02>',
+    })
+
+    assert.is.True(#api.load() > 1)
+    local current_file = cur_file()
+    local headline = current_file.headlines[2]
+    assert.are.same('', headline.priority)
+    assert.are.same('B', config.org_priority_default)
+    config.org_priority_start_cycle_with_default = false
+
+    headline:priority_up():wait()
+    assert.are.same('A', cur_file().headlines[2].priority)
+    assert.is.True(vim.fn.getline(5):match('%[#A%]') ~= nil)
+  end)
+
+  it('should enable priority at default + 1', function()
+    helpers.create_file({
+      '#TITLE: First file',
+      '',
+      '* TODO Test orgmode :WORK:OFFICE:',
+      '  DEADLINE: <2021-07-21 Wed 22:02>',
+      '** TODO Second level :NESTEDTAG:',
+      '  DEADLINE: <2021-07-21 Wed 22:02>',
+      '* DONE Some task',
+      '  DEADLINE: <2021-07-21 Wed 22:02>',
+    })
+
+    assert.is.True(#api.load() > 1)
+    local current_file = cur_file()
+    local headline = current_file.headlines[2]
+    assert.are.same('', headline.priority)
+    assert.are.same('B', config.org_priority_default)
+    config.org_priority_start_cycle_with_default = false
+
+    headline:priority_down():wait()
+    assert.are.same('C', cur_file().headlines[2].priority)
+    assert.is.True(vim.fn.getline(5):match('%[#C%]') ~= nil)
+  end)
+
+  it('should set/unset priorities', function()
+    helpers.create_file({
+      '#TITLE: First file',
+      '',
+      '* TODO Test orgmode :WORK:OFFICE:',
+      '  DEADLINE: <2021-07-21 Wed 22:02>',
+      '** TODO Second level :NESTEDTAG:',
+      '  DEADLINE: <2021-07-21 Wed 22:02>',
+      '* DONE Some task',
+      '  DEADLINE: <2021-07-21 Wed 22:02>',
+    })
+
+    assert.is.True(#api.load() > 1)
+    local current_file = cur_file()
+    local headline = current_file.headlines[2]
+    assert.are.same('', headline.priority)
+
+    cur_file().headlines[2]:set_priority('B'):wait()
+    assert.is.True(vim.fn.getline(5):match('%[#B%]') ~= nil)
     cur_file().headlines[2]:set_priority('C'):wait()
     assert.is.True(vim.fn.getline(5):match('%[#C%]') ~= nil)
     cur_file().headlines[2]:set_priority('A'):wait()

tests/plenary/object/priority_state_spec.lua

@@ -19,7 +19,11 @@ describe('Priority state', function()
   end
 
   local create_priority = function(prio)
-    return PriorityState:new(prio, config:get_priority_range())
+    return PriorityState:new(prio, config:get_priority_range(), true)
+  end
+
+  local create_priority_non_default = function(prio)
+    return PriorityState:new(prio, config:get_priority_range(), false)
   end
 
   it('should increase single numeric priority', function()
@@ -48,28 +52,28 @@ describe('Priority state', function()
     assert.are.same('C', priority:decrease())
   end)
 
-  it('should change to empty priority when numeric increased beyond highest', function()
+  it('should change to lowest priority when numeric increased beyond highest', function()
     numeric_config()
     local priority = create_priority('1')
-    assert.are.same('', priority:increase())
+    assert.are.same('15', priority:increase())
   end)
 
-  it('should change to empty priority when numeric decreased beyond lowest', function()
+  it('should change to highest priority when numeric decreased beyond lowest', function()
     numeric_config()
     local priority = create_priority('15')
-    assert.are.same('', priority:decrease())
+    assert.are.same('1', priority:decrease())
   end)
 
-  it('should change to empty priority when alpha increased beyond highest', function()
+  it('should change to lowest priority when alpha increased beyond highest', function()
     alpha_config()
     local priority = create_priority('A')
-    assert.are.same('', priority:increase())
+    assert.are.same('D', priority:increase())
   end)
 
-  it('should change to empty priority when alpha decreased beyond lowest', function()
+  it('should change to highest priority when alpha decreased beyond lowest', function()
     alpha_config()
     local priority = create_priority('D')
-    assert.are.same('', priority:decrease())
+    assert.are.same('A', priority:decrease())
   end)
 
   it('should convert numeric priorities to a string for comparison', function()
@@ -100,25 +104,37 @@ describe('Priority state', function()
     alpha_config()
     local higher = create_priority('A')
     local lower = create_priority('B')
-    assert.Is.True(higher:get_sort_value() > lower:get_sort_value())
+    assert.is.True(higher:get_sort_value() > lower:get_sort_value())
   end)
 
   it('should compare numeric priorities correctly', function()
     numeric_config()
     local higher = create_priority(1)
     local lower = create_priority(2)
-    assert.Is.True(higher:get_sort_value() > lower:get_sort_value())
+    assert.is.True(higher:get_sort_value() > lower:get_sort_value())
   end)
 
-  it('should change to highest priority if priority increased and currently empty', function()
+  it('should change to default priority if priority increased and currently empty', function()
     alpha_config()
     local priority = create_priority('')
-    assert.are.same('D', priority:increase())
+    assert.are.same('C', priority:increase())
   end)
 
-  it('should change to lowest priority if priority decreased and currently empty', function()
+  it('should change to default priority if priority decreased and currently empty', function()
     alpha_config()
     local priority = create_priority('')
-    assert.are.same('A', priority:decrease())
+    assert.are.same('C', priority:decrease())
+  end)
+
+  it('should change to default + 1 priority if priority increased and currently empty', function()
+    alpha_config()
+    local priority = create_priority_non_default('')
+    assert.are.same('B', priority:increase())
+  end)
+
+  it('should change to default - 1 priority if priority decreased and currently empty', function()
+    alpha_config()
+    local priority = create_priority_non_default('')
+    assert.are.same('D', priority:decrease())
   end)
 end)