-
-
Notifications
You must be signed in to change notification settings - Fork 160
feat(cookie): implement virtual cookies #992
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
base: master
Are you sure you want to change the base?
Conversation
9f8b4d8
to
2de91e0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
It works, but I expected it to work differently. I assumed it will always show a virtual text, not only if you add [/]
or [%]
. And also expected to use a default "eol" virtual text that will show on the right of text, like diagnostics do.
I understand it would look strange if we would have both "regular text cookie" and "virtual text cookie" one beside another, but concealing the current cookie looks a bit odd. A slight improvement would be to not conceal the text and use overlay
virtual text.
I'm not really a user of the cookies, so I'd like to understand your usage of it. Do you need to have the actual cookie text for later use (exporting or something else), or it's only visual? Are there situations where you want cookies somewhere but not on other places?
Underlying "text cookie" and "virtual text cookie" are not in sync, as we discussed on the issue. Underlying cookie updates when you toggle the todo state, but not when you manually edit the TODO
to DONE
. This might cause confusion.
To summarize: I'd prefer if we would have an always visible virtual text on the headlines/checkboxes where applicable, when this setting is turned on, or when enabled through global variable (nice addition btw), and if headline/checkbox have both, just show them both.
Let me know what you think.
I avoided that purely because it can be difficult to see a cookie when we have a bunch of tags on a given headline.
Without the conceal it can be difficult to actually delete the underlying cookie as it's unclear what is actually being selected/modified if the cookie is overlayed and a user wants to edit the underlying cookie text. In my mind we only want to show the virtual cookie and suppress the "real" cookie when possible as the virtual cookie gets live updates and will be in sync with the actual state of the headlines.
It's purely visual for me for quickly tracking progress on a larger Again, an issue that can crop up is if we overlay the cookie with a virtual cookie, it can be difficult to modify the underlying cookie text. That was why I have this setup as it currently is. Here's what I'll do:
Here's what I need answered before I can continue on some of this:
|
Latest push covers
As well as
|
Updated documentation to reflect the new changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work!
I tested this locally and found a single bug. The behavior of the "real" cookie being hidden in favor of the virtual one looks reasonable to me.
Thank you for the effort on this. I appreciate the feature.
Virtual text remains rendered when parent heading is deleted
Kazam_screencast_00021.mp4
*** virt_cookies | ||
:PROPERTIES: | ||
:CUSTOM_ID: virt_cookies | ||
:END: | ||
|
||
You can toggle Virtual cookies on the fly by executing command =:Org cookie_mode= when in a org buffer. | ||
This additionally sets the buffer variable =vim.b.org_cookie_mode= to =true= or =false=, depending on the current state. | ||
|
||
Currently this only applies Virtual cookies to headlines. | ||
|
||
Uses the following highlights: | ||
- [email protected]~: The highlight to use for the delimiters (brackets: ~[~ & ~]~) | ||
- [email protected]~: A more granular highlight for just the left ~[~ bracket | ||
- [email protected]~: A more granular highlight for just the right ~]~ bracket | ||
- [email protected]~: The numbers used in the cookie (~50~) | ||
- ~org.cookie.number.complete~: A more granular highlight for the left side of ~[1/5]~ (in this case the ~1~) | ||
- ~org.cookie.number.total~: A more granular highlight for the right side of ~[1/5]~ (in this case the ~5~) | ||
- [email protected]~: The sign used in the cookie (e.g. ~/~, ~%~, or ~???~) | ||
- [email protected]~: More granular for the div sign, the ~/~ in ~[1/5]~ | ||
- [email protected]~: More granular for the percent sign, the ~%~ in ~[100%]~ | ||
|
||
**** enabled | ||
:PROPERTIES: | ||
:CUSTOM_ID: virt_cookies_enabled | ||
:END: | ||
|
||
- Type: =boolean= | ||
- Default: =false= | ||
Possible values: | ||
- =true= - Uses /Virtual/ cookies to show live progress for applicable headlines. | ||
- =false= - Do not add any /Virtual/ cookies. | ||
|
||
**** type | ||
:PROPERTIES: | ||
:CUSTOM_ID: virt_cookies_type | ||
:END: | ||
|
||
- Type: ='%' | '/'= | ||
- Default: ='/'= | ||
|
||
The type of virtual cookie to draw for headlines that do not contain a "real" cookie. If a headline contains a real cookie, then it will opt to use the cookie type from that real cookie. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of things came to mind on this docs entry.
- We could use a
#+begin_src lua
code block to show an example for this config, same as done with the other config options. Something like this:
diff --git a/docs/configuration.org b/docs/configuration.org
index 899c3df..91f01f7 100644
--- a/docs/configuration.org
+++ b/docs/configuration.org
@@ -3018,9 +3018,22 @@ require('orgmode').setup({
:CUSTOM_ID: virt_cookies
:END:
+Virtual cookies display the progress of TODO headings with children by indicating the proportion of DONE children.
+
You can toggle Virtual cookies on the fly by executing command =:Org cookie_mode= when in a org buffer.
This additionally sets the buffer variable =vim.b.org_cookie_mode= to =true= or =false=, depending on the current state.
+#+begin_src lua
+require('orgmode').setup({
+ ui = {
+ virt_cookies = {
+ enabled = true,
+ type = '/',
+ },
+ }
+})
+#+end_src
+
Currently this only applies Virtual cookies to headlines.
Uses the following highlights:
- We are missing an entry in
doc/orgmode.txt
for this config option - The individual config properties (
type
andenabled
) should probably not be described in individual child headings (****
), but instead in list items, similar to what is done here for theorg_capture_templates
config options:
orgmode/docs/configuration.org
Lines 832 to 842 in 32ef9e9
- =description= (=string=) --- description of the template that is displayed in the template selection menu - =template= (=string|string[]=) --- body of the template that will be used when creating capture - =target= (=string?=) --- name of the file to which the capture content will be added. If the target is not specified, the content will be added to the [[#org_default_notes_file][org_default_notes_file]] file - =headline= (=string?=) --- title of the headline after which the capture content will be added. If no headline is specified, the content will be appended to the end of the file - =datetree (boolean | { time_prompt?: boolean, reversed?: boolean, tree_type: 'day' | 'month' | 'week' | 'custom' })=
---@type OrgCookieWatchers | ||
local watchers = {} | ||
|
||
---Get all currently registered cookie watchers | ||
---@return OrgCookieWatchers | ||
function OrgVirtCookie.watchers() | ||
return watchers | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question
Is there a particular reason for the watchers not to hang from the OrgVirtCookie
class itself?
I've not done much with lua, so not sure if there is much of a difference in the approaches.
I have in mind something like this:
diff --git a/ftplugin/org.lua b/ftplugin/org.lua
index 6bc4213..b5c82b5 100644
--- a/ftplugin/org.lua
+++ b/ftplugin/org.lua
@@ -19,7 +19,7 @@ if config.org_startup_indented then
end
if config.ui.virt_cookies.enabled then
- require('orgmode.ui.virtcookie').new(bufnr, config.ui.virt_cookies.type):attach()
+ require('orgmode.ui.virtcookie'):new(bufnr, config.ui.virt_cookies.type):attach()
end
vim.bo.modeline = false
diff --git a/lua/orgmode/org/global.lua b/lua/orgmode/org/global.lua
index f2879f0..5f0c260 100644
--- a/lua/orgmode/org/global.lua
+++ b/lua/orgmode/org/global.lua
@@ -99,13 +99,13 @@ local build = function(orgmode)
require('orgmode.ui.virtual_indent').toggle_buffer_indent_mode()
end,
cookie_mode = function()
- local virtcookie = require('orgmode.ui.virtcookie').get()
+ local virtcookie = require('orgmode.ui.virtcookie'):get()
if virtcookie then
virtcookie:toggle()
end
end,
cookie_type = function()
- local virtcookie = require('orgmode.ui.virtcookie').get()
+ local virtcookie = require('orgmode.ui.virtcookie'):get()
if virtcookie then
virtcookie:toggle_cookie_type()
end
diff --git a/lua/orgmode/ui/virtcookie.lua b/lua/orgmode/ui/virtcookie.lua
index c9928fd..ab08531 100644
--- a/lua/orgmode/ui/virtcookie.lua
+++ b/lua/orgmode/ui/virtcookie.lua
@@ -1,6 +1,7 @@
local org = require('orgmode')
---@alias OrgVirtCookieType '/' | '%' The type of cookie to default to when no "real" cookie exists
+---@alias OrgCookieWatchers table<integer, OrgVirtCookie> A mapping of buffer ids to watchers
---@class OrgVirtCookie Updates Headline Cookies for Progress using Virtual Text
---@field private bufnr integer Buffer Watcher is attached to
@@ -9,34 +10,31 @@ local org = require('orgmode')
---@field private ns_id integer
local OrgVirtCookie = {
ns_id = vim.api.nvim_create_namespace('orgmode.ui.cookie'),
+ ---@type OrgCookieWatchers
+ watchers = {},
}
----@alias OrgCookieWatchers table<integer, OrgVirtCookie> A mapping of buffer ids to watchers
-
----@type OrgCookieWatchers
-local watchers = {}
-
---Get all currently registered cookie watchers
---@return OrgCookieWatchers
-function OrgVirtCookie.watchers()
- return watchers
+function OrgVirtCookie:getWatchers()
+ return self.watchers
end
---Gets an existing OrgCookieWatcher for the given buffer if it exists
---@param bufnr? integer Buffer to get the watcher for
---@return OrgVirtCookie?
-function OrgVirtCookie.get(bufnr)
+function OrgVirtCookie:get(bufnr)
bufnr = bufnr or vim.api.nvim_get_current_buf()
- local watcher = OrgVirtCookie.watchers()[bufnr]
+ local watcher = OrgVirtCookie:getWatchers()[bufnr]
return watcher
end
---Creates a new headline watcher
---@param bufnr? integer Buffer to watch, if unspecified then uses the current buffer
---@param cookie_type? OrgVirtCookieType
-function OrgVirtCookie.new(bufnr, cookie_type)
+function OrgVirtCookie:new(bufnr, cookie_type)
bufnr = bufnr or vim.api.nvim_get_current_buf()
- local watcher = OrgVirtCookie.get(bufnr)
+ local watcher = OrgVirtCookie:get(bufnr)
if watcher then
return watcher
end
@@ -45,7 +43,7 @@ function OrgVirtCookie.new(bufnr, cookie_type)
attached = false,
cookie_type = cookie_type or '/',
}, { __index = OrgVirtCookie })
- watchers[this.bufnr] = this
+ self.watchers[this.bufnr] = this
vim.api.nvim_create_autocmd('BufDelete', {
buffer = this.bufnr,
@@ -55,7 +53,7 @@ function OrgVirtCookie.new(bufnr, cookie_type)
end,
})
- return watchers[this.bufnr]
+ return self.watchers[this.bufnr]
end
---@param new_cookie_type OrgVirtCookieType
@@ -251,7 +249,7 @@ function OrgVirtCookie:attach()
vim.b[self.bufnr].org_cookie_mode = true
vim.b[self.bufnr].org_cookie_type = self.cookie_type
- watchers[self.bufnr] = self
+ self.watchers[self.bufnr] = self
self:_update_cookies_in_range(0, vim.api.nvim_buf_line_count(self.bufnr) - 1)
-- We cant use `nvim_set_decoration_provider()` here because of a
@@ -314,7 +312,7 @@ end
---Completely removes the watcher, including from tracked watchers
function OrgVirtCookie:delete()
self:detach()
- watchers[self.bufnr] = nil
+ self.watchers[self.bufnr] = nil
end
return OrgVirtCookie
function OrgVirtCookie._parent_headlines(headline) | ||
local located_headlines = {} | ||
local count = 0 | ||
while true do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a scenario where this would go infinite, but I believe you can avoid while true
with a do while
(repeat until
in lua)
diff --git a/lua/orgmode/ui/virtcookie.lua b/lua/orgmode/ui/virtcookie.lua
index c9928fd..c2b239b 100644
--- a/lua/orgmode/ui/virtcookie.lua
+++ b/lua/orgmode/ui/virtcookie.lua
@@ -82,7 +82,8 @@ end
function OrgVirtCookie._parent_headlines(headline)
local located_headlines = {}
local count = 0
- while true do
+
+ repeat
count = count + 1
local parent = headline:get_parent_headline()
@@ -92,7 +93,8 @@ function OrgVirtCookie._parent_headlines(headline)
table.insert(located_headlines, parent)
headline = parent
- end
+ until not parent or not parent.headline
+
return located_headlines
end
|
||
-- For cookie extmarks (applicable if enabled) | ||
['@org.cookie.delimiter'] = 'Delimiter', | ||
['@org.cookie.sign'] = 'Special', | ||
['@org.cookie.number'] = 'Number', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These do look good, but the highlights make it seem like it is real text rendered in the buffer.
The UX may be more consistent if regular virtual text highlights are used instead.
diff --git a/lua/orgmode/colors/highlights.lua b/lua/orgmode/colors/highlights.lua
index 3601119..2885cdb 100644
--- a/lua/orgmode/colors/highlights.lua
+++ b/lua/orgmode/colors/highlights.lua
@@ -73,9 +73,9 @@ function M.link_highlights()
['@org.edit_src'] = 'Visual',
-- For cookie extmarks (applicable if enabled)
- ['@org.cookie.delimiter'] = 'Delimiter',
- ['@org.cookie.sign'] = 'Special',
- ['@org.cookie.number'] = 'Number',
+ ['@org.cookie.delimiter'] = 'VirtualTextInfo',
+ ['@org.cookie.sign'] = 'VirtualTextInfo',
+ ['@org.cookie.number'] = 'VirtualTextInfo',
}
for src, def in pairs(links) do
Summary
This PR adds virtual cookies, a way of using extmarks to get live updating cookies for progress.
Currently it is only implemented for headlines, does not support list items.
This slightly modifies the headlines class to expose progress amounts from the headlines.
The virtual cookies prioritize list checkboxes over todo checkboxes for determining progress. Would it be preferred to flip that OR combine the totals from both as our progress?
Virtual cookies are gated behind
ui.cookies_use_extmarks
and are disabled by default.I have not added tests for them, I consider them largely experimental (though pretty robust from my experience) and didn't allocate the time to write tests for them.
See more details from my earlier comment (#745 (comment)) for additional information.
Related Issues
Closes #745
Changes
Headline
classconfiguration.org
Org cookie_mode
command for toggling cookies on & off in the current bufferChecklist
I confirm that I have:
Conventional Commits
specification (e.g.,
feat: add new feature
,fix: correct bug
,docs: update documentation
).make test
. (Ensured existing tests still pass)