Skip to content
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

feat: Headline:update_cookie() now works for both list and headline children #572

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lua/orgmode/org/mappings.lua
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,7 @@ function OrgMappings:handle_return(suffix)
local content = config:respect_blank_before_new_entry({ string.rep('*', item.level) .. ' ' .. suffix })
vim.fn.append(linenr, content)
vim.fn.cursor(linenr + #content, 0)
require('orgmode.treesitter.headline'):new(item.node):update_cookie()
return vim.cmd([[startinsert!]])
end

Expand Down
70 changes: 57 additions & 13 deletions lua/orgmode/treesitter/headline.lua
Original file line number Diff line number Diff line change
Expand Up @@ -205,12 +205,15 @@ function Headline:set_todo(keyword)
local current_todo = self:todo()
if current_todo then
tree_utils.set_node_text(current_todo, keyword)
self:update_cookie()
return
end

local stars = self:stars()
local text = ts.get_node_text(stars, 0)
tree_utils.set_node_text(stars, string.format('%s %s', text, keyword))
self:refresh()
self:update_cookie()
Comment on lines +215 to +216
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this necessary? And if we update_cookie() here, is there even a need to call it in L208?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The L208 call is necessary because of the return statement at L209. The code was written that way when I added the calls to update_cookie(). Prior to my PR, I think the function worked as follows: if the headline is a todo item already, set the node text to keyword and then return; if not, insert the TODO keyword into the node. self:update_cookie() needs to be called after both changes, and there's probably a way to refactor the function to only call it once, but I didn't feel qualified to rewrite the original logic of the function in this PR.

As to the other question about self.refresh(): the short answer is "because it doesn't work without it." I don't 100% know why the call to self:refresh() is necessary at L215, but if it isn't there, the value of the headline is wrong when self:update_cookie() is called. I suspect it's because tree_utils.set_node_text() uses vim.api.nvim_buf_set_text() to update the node and something in self:refresh() causes treesitter to get back in sync so that the calls to the treesitter API in self:update_cookie() return the correctly updated value. It took my over an hour of debugging to settle on self:refresh() here and I tried several other solutions (including vim.schedule_wrap() and vim.defer_fn()), so I'm pretty confident it has to be called here (unless there's some other solution I didn't try). But I agree with the original question: that call to self:refresh() looks wrong.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because of the return statement at L209

🤦‍♂️ of course, my bad

because it doesn't work without it

Let's figure out what is going on and then document the code

end

function Headline:item()
Expand Down Expand Up @@ -436,22 +439,63 @@ function Headline:cookie()
return self:parse('%[%d?%d?%d?%%%]')
end

function Headline:update_cookie(list_node)
local total_boxes = self:child_checkboxes(list_node)
local checked_boxes = vim.tbl_filter(function(box)
return box:match('%[%w%]')
end, total_boxes)

local cookie = self:cookie()
if cookie then
local new_cookie_val
if ts.get_node_text(cookie, 0):find('%%') then
new_cookie_val = ('[%d%%]'):format((#checked_boxes / #total_boxes) * 100)
function Headline:update_cookie()
local total = 0
local done = 0
local target = self

-- Determine the target (headline with the cookie). This could be parent
-- headline, as Headline:set_todo() will likely be called on headlines
-- whose parents have cookies.
local parent_section = tree_utils.find_parent_type(target.headline, 'section')
local cookie = target:cookie()
if not cookie then
-- We need to check the next section up:
parent_section = target.headline:parent():parent()
if parent_section:child(0):type() == 'headline' then
local parent_headline = Headline:new(parent_section:child(0))
cookie = parent_headline:cookie()
if cookie ~= nil then
target = parent_headline
else
return nil
end
else
new_cookie_val = ('[%d/%d]'):format(#checked_boxes, #total_boxes)
return nil
end
tree_utils.set_node_text(cookie, new_cookie_val)
end

-- Parse the children of the headline's parent section for child headlines and lists:
for _, node in pairs(ts_utils.get_named_children(parent_section)) do
-- The child is a list:
if node:type() == 'body' and node:child(0):type() == 'list' then
local total_boxes = target:child_checkboxes(node:child(0))
local checked_boxes = vim.tbl_filter(function(box)
return box:match('%[%w%]')
end, total_boxes)
total = total + #total_boxes
done = done + #checked_boxes
end
-- The child is a section:
if node:type() == 'section' and node:child(0):type() == 'headline' then
local hl = Headline:new(node:child(0))
local _, word, is_done = hl:todo()
if word ~= nil then
total = total + 1
if is_done then
done = done + 1
end
end
end
end

local new_cookie_val
if ts.get_node_text(cookie, 0):find('%%') then
new_cookie_val = ('[%d%%]'):format((total == 0 and 0 or done / total) * 100)
else
new_cookie_val = ('[%d/%d]'):format(done, total)
end
tree_utils.set_node_text(cookie, new_cookie_val)
end

function Headline:child_checkboxes(list_node)
Expand Down
2 changes: 1 addition & 1 deletion lua/orgmode/treesitter/listitem.lua
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ function Listitem:update_checkbox(action)
else
local parent_headline = tree_utils.closest_headline()
if parent_headline then
Headline:new(parent_headline):update_cookie(parent_list)
Headline:new(parent_headline):update_cookie()
end
end
end
Expand Down