Fix priorities (#784) (#785)

* test: implement test to expose the bug

* test: add more cases in priority_state_spec

* refactor: make priority_state independent of config

To prepare the fix, we need to avoid a dependency cycle between config
and priority_state, because we will need priority_state in config to
implement the fix.

We inject the needed values, whereever an instance of PriorityState is
created.

* fix: generate missing intermediate priorities

A headline priority gets only detected correctly, if it is part of the
table returned by Config:get_priorities(). While it previously only
returned the priorities explicitly defined by the user, it now uses
PriorityState to generate also intermediate ones. This happens, if the
highest and the lowest priority define a range of more than three
priorities.

* docs: mention new highlight groups

Add new highlight groups to "Colors" chapter.

---------

Co-authored-by: Sebastian Flügge <[email protected]>

Author: seflue

DOCS.md

@@ -1708,7 +1708,9 @@ The following highlight groups are used:
 - `@org.headline.level7`: Headline at level 7 - linked to `Special`
 - `@org.headline.level8`: Headline at level 8 - linked to `String`
 - `@org.priority.highest`: Highest priority marker - linked to `@comment.error`
+- `@org.priority.high`: High priority marker - Not linked to anything, defaults to normal text
 - `@org.priority.default`: Default priority marker - Not linked to anything, defaults to normal text
+- `@org.priority.low`: Lowest priority marker - Not linked to anything, defaults to normal text
 - `@org.priority.lowest`: Lowest priority marker - Not linked to anything, defaults to normal text
 - `@org.timestamp.active`: An active timestamp - linked to `@keyword`
 - `@org.timestamp.inactive`: An inactive timestamp - linked to `@comment`

lua/orgmode/api/headline.lua

@@ -1,4 +1,5 @@
 local OrgPosition = require('orgmode.api.position')
+local config = require('orgmode.config')
 local PriorityState = require('orgmode.objects.priority_state')
 local Date = require('orgmode.objects.date')
 local Calendar = require('orgmode.objects.calendar')
@@ -112,7 +113,8 @@ function OrgHeadline:priority_up()
   return self:_do_action(function()
     local headline = org.files:get_closest_headline()
     local current_priority = headline:get_priority()
-    local priority_state = PriorityState:new(current_priority)
+    local prio_range = config:get_priority_range()
+    local priority_state = PriorityState:new(current_priority, prio_range)
     return headline:set_priority(priority_state:increase())
   end)
 end
@@ -123,7 +125,8 @@ function OrgHeadline:priority_down()
   return self:_do_action(function()
     local headline = org.files:get_closest_headline()
     local current_priority = headline:get_priority()
-    local priority_state = PriorityState:new(current_priority)
+    local prio_range = config:get_priority_range()
+    local priority_state = PriorityState:new(current_priority, prio_range)
     return headline:set_priority(priority_state:decrease())
   end)
 end

lua/orgmode/config/init.lua

@@ -5,6 +5,7 @@ local defaults = require('orgmode.config.defaults')
 ---@type table<string, OrgMapEntry>
 local mappings = require('orgmode.config.mappings')
 local TodoKeywords = require('orgmode.objects.todo_keywords')
+local PriorityState = require('orgmode.objects.priority_state')
 
 ---@class OrgConfig:OrgDefaultConfig
 ---@field opts table
@@ -331,12 +332,37 @@ function Config:get_inheritable_tags(headline)
   end, headline.tags)
 end
 
-function Config:get_priorities()
+function Config:get_priority_range()
   return {
+    highest = self.org_priority_highest,
+    default = self.org_priority_default,
+    lowest = self.org_priority_lowest,
+  }
+end
+
+function Config:get_priorities()
+  local priorities = {
     [self.opts.org_priority_highest] = { type = 'highest', hl_group = '@org.priority.highest' },
-    [self.opts.org_priority_default] = { type = 'default', hl_group = '@org.priority.default' },
-    [self.opts.org_priority_lowest] = { type = 'lowest', hl_group = '@org.priority.lowest' },
   }
+
+  local current_prio = PriorityState:new(self.opts.org_priority_highest, self:get_priority_range())
+  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' }
+  end
+
+  -- we need to overwrite the default value set by the first loop
+  priorities[self.opts.org_priority_default] = { type = 'default', hl_group = '@org.priority.default' }
+
+  while current_prio:as_num() < current_prio:lowest_as_num() do
+    current_prio:decrease()
+    priorities[current_prio.priority] = { type = 'low', hl_group = '@org.priority.low' }
+  end
+
+  -- we need to overwrite the lowest value set by the second loop
+  priorities[self.opts.org_priority_lowest] = { type = 'lowest', hl_group = '@org.priority.lowest' }
+
+  return priorities
 end
 
 function Config:setup_ts_predicates()

lua/orgmode/files/headline.lua

@@ -192,7 +192,8 @@ end
 
 function Headline:get_priority_sort_value()
   local priority = self:get_priority()
-  return PriorityState:new(priority):get_sort_value()
+  local prio_range = config:get_priority_range()
+  return PriorityState:new(priority, prio_range):get_sort_value()
 end
 
 function Headline:is_archived()

lua/orgmode/objects/priority_state.lua

@@ -1,4 +1,3 @@
-local config = require('orgmode.config')
 local utils = require('orgmode.utils')
 
 ---@class OrgPriorityState
@@ -9,12 +8,13 @@ local utils = require('orgmode.utils')
 local PriorityState = {}
 
 ---@param priority string
-function PriorityState:new(priority)
+---@param prio_range { highest: string, lowest: string, default: string }
+function PriorityState:new(priority, prio_range)
   local o = {}
 
-  o.high_priority = tostring(config.org_priority_highest)
-  o.low_priority = tostring(config.org_priority_lowest)
-  o.default_priority = tostring(config.org_priority_default)
+  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)
 
   setmetatable(o, self)
@@ -78,13 +78,44 @@ function PriorityState:decrease()
   return self.priority
 end
 
+function PriorityState:highest_as_num()
+  return PriorityState._as_number(self.high_priority)
+end
+
+function PriorityState:default_as_num()
+  return PriorityState._as_number(self.default_priority)
+end
+
+function PriorityState:lowest_as_num()
+  return PriorityState._as_number(self.low_priority)
+end
+
+function PriorityState:as_num()
+  return PriorityState._as_number(self.priority)
+end
+
 ---@param direction number
 ---@return string
 function PriorityState:_apply(direction)
-  if type(tonumber(self.priority)) == 'number' then
-    return tostring(tonumber(self.priority) + direction)
+  local new_value = PriorityState._as_number(self.priority) + direction
+  if PriorityState._is_number(self.priority) then
+    return tostring(new_value)
+  end
+  return string.char(new_value)
+end
+
+---@param prio string
+---@return number?
+function PriorityState._as_number(prio)
+  if PriorityState._is_number(prio) then
+    return tonumber(prio)
   end
-  return string.char(string.byte(self.priority) + direction)
+  return string.byte(prio)
+end
+
+---@return boolean
+function PriorityState._is_number(prio)
+  return type(tonumber(prio)) == 'number'
 end
 
 return PriorityState

lua/orgmode/org/mappings.lua

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

tests/plenary/object/priority_state_spec.lua

@@ -18,101 +18,107 @@ describe('Priority state', function()
     })
   end
 
+  local create_priority = function(prio)
+    return PriorityState:new(prio, config:get_priority_range())
+  end
+
   it('should increase single numeric priority', function()
     numeric_config()
-    local priority = PriorityState:new(10)
+    local priority = create_priority(10)
     assert.are.same('9', priority:increase())
   end)
 
   it('should decrease single numeric priority', function()
     numeric_config()
-    local priority = PriorityState:new(9)
+    local priority = create_priority(9)
     assert.are.same('10', priority:decrease())
   end)
 
   it('should increase single alpha priority', function()
     alpha_config()
-    local priority = PriorityState:new('C')
+    local priority = create_priority('D')
+    assert.are.same('C', priority:increase())
     assert.are.same('B', priority:increase())
+    assert.are.same('A', priority:increase())
   end)
 
   it('should decrease single alpha priority', function()
     alpha_config()
-    local priority = PriorityState:new('B')
+    local priority = create_priority('B')
     assert.are.same('C', priority:decrease())
   end)
 
   it('should change to empty priority when numeric increased beyond highest', function()
     numeric_config()
-    local priority = PriorityState:new('1')
+    local priority = create_priority('1')
     assert.are.same('', priority:increase())
   end)
 
   it('should change to empty priority when numeric decreased beyond lowest', function()
     numeric_config()
-    local priority = PriorityState:new('15')
+    local priority = create_priority('15')
     assert.are.same('', priority:decrease())
   end)
 
   it('should change to empty priority when alpha increased beyond highest', function()
     alpha_config()
-    local priority = PriorityState:new('A')
+    local priority = create_priority('A')
     assert.are.same('', priority:increase())
   end)
 
   it('should change to empty priority when alpha decreased beyond lowest', function()
     alpha_config()
-    local priority = PriorityState:new('D')
+    local priority = create_priority('D')
     assert.are.same('', priority:decrease())
   end)
 
   it('should convert numeric priorities to a string for comparison', function()
     numeric_config()
-    local priority = PriorityState:new(1)
+    local priority = create_priority(1)
     assert.are.same(priority.priority, '1')
   end)
 
   it('should return the string representation of the value to use for sorting for alpha strings', function()
     alpha_config()
-    local priority = PriorityState:new('A')
+    local priority = create_priority('A')
     assert.are.same(-65, priority:get_sort_value())
   end)
 
   it('should return the string representation of the value to use for sorting for numeric strings', function()
     numeric_config()
-    local priority = PriorityState:new(1)
+    local priority = create_priority(1)
     assert.are.same(-49, priority:get_sort_value())
   end)
 
   it('should return default priority value if empty when sorting', function()
     alpha_config()
-    local priority = PriorityState:new('')
+    local priority = create_priority('')
     assert.are.same(-1 * string.byte(config.org_priority_default), priority:get_sort_value())
   end)
 
   it('should compare alpha priorities correctly', function()
     alpha_config()
-    local higher = PriorityState:new('A')
-    local lower = PriorityState:new('B')
+    local higher = create_priority('A')
+    local lower = create_priority('B')
     assert.Is.True(higher:get_sort_value() > lower:get_sort_value())
   end)
 
   it('should compare numeric priorities correctly', function()
     numeric_config()
-    local higher = PriorityState:new(1)
-    local lower = PriorityState:new(2)
+    local higher = create_priority(1)
+    local lower = create_priority(2)
     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()
     alpha_config()
-    local priority = PriorityState:new('')
+    local priority = create_priority('')
     assert.are.same('D', priority:increase())
   end)
 
   it('should change to lowest priority if priority decreased and currently empty', function()
     alpha_config()
-    local priority = PriorityState:new('')
+    local priority = create_priority('')
     assert.are.same('A', priority:decrease())
   end)
 end)

tests/plenary/ui/mappings/priority_spec.lua

@@ -1,6 +1,23 @@
+local config = require('orgmode.config')
 local helpers = require('tests.plenary.helpers')
 
 describe('Priority mappings', function()
+  local alpha_config = function()
+    config:extend({
+      org_priority_highest = 'A',
+      org_priority_default = 'C',
+      org_priority_lowest = 'E',
+    })
+  end
+
+  local numeric_config = function()
+    config:extend({
+      org_priority_highest = 1,
+      org_priority_default = 5,
+      org_priority_lowest = 15,
+    })
+  end
+
   after_each(function()
     vim.cmd([[silent! %bw!]])
   end)
@@ -64,4 +81,48 @@ describe('Priority mappings', function()
     vim.cmd('norm ,o,a\r')
     assert.are.same('* [#A] Test orgmode', vim.fn.getline(1))
   end)
+
+  it('should increase the default character priority, when it is explicitly defined', function()
+    alpha_config()
+    helpers.create_file({
+      '* [#C] Test orgmode',
+    })
+    vim.fn.cursor(1, 1)
+    assert.are.same('* [#C] Test orgmode', vim.fn.getline(1))
+    vim.cmd('norm ciR')
+    assert.are.same('* [#B] Test orgmode', vim.fn.getline(1))
+  end)
+
+  it('should increase a numeric priority, which is not explicitly defined', function()
+    numeric_config()
+    helpers.create_file({
+      '* [#5] Test orgmode',
+    })
+    vim.fn.cursor(1, 1)
+    assert.are.same('* [#5] Test orgmode', vim.fn.getline(1))
+    vim.cmd('norm ciR')
+    assert.are.same('* [#4] Test orgmode', vim.fn.getline(1))
+  end)
+
+  it('should increase a character priority, which is not explicitly defined', function()
+    alpha_config()
+    helpers.create_file({
+      '* [#D] Test orgmode',
+    })
+    vim.fn.cursor(1, 1)
+    assert.are.same('* [#D] Test orgmode', vim.fn.getline(1))
+    vim.cmd('norm ciR')
+    assert.are.same('* [#C] Test orgmode', vim.fn.getline(1))
+  end)
+
+  it('should increase a numeric priority, which is not explicitly defined', function()
+    numeric_config()
+    helpers.create_file({
+      '* [#10] Test orgmode',
+    })
+    vim.fn.cursor(1, 1)
+    assert.are.same('* [#10] Test orgmode', vim.fn.getline(1))
+    vim.cmd('norm ciR')
+    assert.are.same('* [#9] Test orgmode', vim.fn.getline(1))
+  end)
 end)