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

[plugin] org-roam.nvim progress & discussion #702

Closed
chipsenkbeil opened this issue Mar 15, 2024 · 158 comments
Closed

[plugin] org-roam.nvim progress & discussion #702

chipsenkbeil opened this issue Mar 15, 2024 · 158 comments
Labels
bug Something isn't working

Comments

@chipsenkbeil
Copy link
Contributor

chipsenkbeil commented Mar 15, 2024

About this task

This issue started discussing a potential bug, and ended up evolving into a rundown of the ongoing work on org-roam.nvim, a plugin to implement Org-roam in neovim.

Scroll further down to see details about the plugin, code pointers and highlights, etc.

The original bug report is kept below for clarity.

Describe the bug

I am writing a plugin that creates some buffers with a buftype of nofile. The purpose of these is to generate some immutable orgmode content that you can navigate. In particular, I want to take advantage of the fantastic folding that orgmode offers.

Unfortunately, when I go to collapse a section, I get an error about the current file is not found or not an org file, caused by

assert(orgfile, 'Current file not found or not an org file')

Steps to reproduce

  1. Create a scratch buffer via vim.api.nvim_create_buf(false, true)
  2. Set the filetype to org via vim.api.nvim_buf_set_option(bufnr, "filetype", "org")
  3. Populate with some org headings via vim.api.nvim_buf_set_lines(bufnr, 0, -1, true, {"* Heading 1-a", "** Heading 2", "* Heading 1-b"})
  4. Navigate to the buffer and hit tab to try to collapse or expand a section

Expected behavior

Section is properly folded.

Emacs functionality

No response

Minimal init.lua

-- Import orgmode using the minimal init example

Screenshots and recordings

No response

OS / Distro

MacOS 14.4

Neovim version/commit

0.9.5

Additional context

Using orgmode on master branch at commit 651078a.

@chipsenkbeil chipsenkbeil added the bug Something isn't working label Mar 15, 2024
@kristijanhusak
Copy link
Member

File is considered valid only if it has an org or org_archive extension. Try setting a name for a buffer that has an org extension with nvim_buf_set_name

  local bufnr = vim.api.nvim_create_buf(false, true)
  vim.api.nvim_buf_set_option(bufnr, 'filetype', 'org')
  vim.api.nvim_buf_set_name(bufnr, 'somename.org')
  vim.api.nvim_buf_set_lines(bufnr, 0, -1, true, { '* Heading 1-a', '** Heading 2', '* Heading 1-b' })
  vim.cmd('b'..bufnr)

@chipsenkbeil
Copy link
Contributor Author

chipsenkbeil commented Mar 15, 2024

@kristijanhusak that solved the error about not being a current file! I'm hitting an issue where the plugin still doesn't detect a fold. There may be something wrong with my buffer or setup. Applies on any heading.

image

@kristijanhusak
Copy link
Member

What would you expect to be folded here? Did you try doing zx to recalculate folds?
I see you are trying to implement org-roam. I had similar idea but didn't catch time to start working on it.

@chipsenkbeil
Copy link
Contributor Author

chipsenkbeil commented Mar 15, 2024

The first heading. If I create a file manually and reproduce the contents, I can fold it.

image

I had to make the repo private while I got permission to work on it on personal time. Now that I have it, here's the current plugin: https://github.com/chipsenkbeil/org-roam.nvim

About the plugin

Database

I wrote a simplistic implementation of a database with indexing supported that provides an easy search for links and backlinks. It isn't as full-fledged as SQL and I'm sure will struggle with larger roam collections, but for me this is enough.

Parser

Leveraging the orgmode treesitter parser for me to find the details I need to build the above database.

Completion of node under cursor

Covers both completing within a link and under a cursor. Essentially does the complete everywhere out of the box. Here's a demo:

example-org-roam-completion.mp4

Quickfix jump to backlinks

I like the quickfix list, so while Emacs doesn't have this and uses an org-roam buffer, this was easy to whip up for neovim:

example-org-roam-quickfix.mp4

Org buffer

I was generating an org file instead of writing a custom buffer. I may switch to the custom buffer highlighting and format anyway because I need to capture link navigation to open in a different buffer versus the current one.

Id navigation

Doesn't seem to work for me even though I thought it was introduced into the plugin somewhat recently. So I'll need to write an org-roam variant to support opening id links. Shouldn't be hard at all. I also built out id generation for a uuid-v4 format in pure Lua. My test nodes aren't using it, though.

image

@kristijanhusak
Copy link
Member

Thanks for opening it up. I just skimmed through it, and it seems you did a lot of custom stuff.
A recent refactor was also done to support org-roam with some of the internals. This is what I had in mind:

  1. Use orgmode.files to load org-roam specific directories https://github.com/nvim-orgmode/orgmode/blob/master/lua/orgmode/init.lua#L51
  2. Use OrgFile:get_links() to collect all links instead of using a database. I'm not sure if this would work though
  3. Add a custom source for completion through the orgmode internals like it is done for everything else here https://github.com/nvim-orgmode/orgmode/blob/master/lua/orgmode/org/autocompletion/init.lua#L23
  4. Use orgmode capture class with custom templates only for org-roam where it would append the properties with id

Doesn't seem to work for me even though I thought it was introduced into the plugin somewhat recently

In the issue description you wrote you are using commit 651078a. That's a fairly old one, and id was not supported there yet. To generate ids you can use this class https://github.com/nvim-orgmode/orgmode/blob/master/lua/orgmode/org/id.lua

Not sure if this information changes anything for you, but I planned to do this. I generally wouldn't suggest using internal classes, but I planned to do that since everything would be part of the same organization.

@chipsenkbeil
Copy link
Contributor Author

chipsenkbeil commented Mar 15, 2024

Thanks for the pointers! When I looked at the plugin's API and data structures, it was (and I believe still is) missing crucial information I'd need for a fully-functional org-roam implementation. Would it make sense to move this discussion to a separate issue? I know you have the plugin API pinned issue, but there's a lot of different questions and dialog I'd want to have about needs and usage that feels better for a separate issue.

Use orgmode.files to load org-roam specific directories https://github.com/nvim-orgmode/orgmode/blob/master/lua/orgmode/init.lua#L51

Haven't looked at this. I wanted to be fairly flexible in the API used to load files, and I also needed some information I didn't see a couple of months back such as top-level drawer access, location information regarding where links/headings/properties are, detection of links/nodes under cursor, etc.

Wonder how much of that has changed or was misinformed from my first look.

I also didn't check to see how you're loading files. I tried to keep most of my logic async - in the sense of leveraging neovim's scheduler and callbacks - so I can (in the future) easily report loading a large set of files, monitoring file changes and reloading, etc.

Use OrgFile:get_links() to collect all links instead of using a database. I'm not sure if this would work though

I don't know on this one. The database I wrote is a way to both track outgoing links (ones you'd find in a file) and incoming links (aka backlinks into a file). I like the design I've got, so I'll most likely keep this.

Populating the database, on the other hand, could definitely switch from a custom parser to what you've got, but to my understanding your links and other structures do not capture their location within a file, which I need in order to show where backlinks, citations, and unlinked references come from.

Add a custom source for completion through the orgmode internals like it is done for everything else here https://github.com/nvim-orgmode/orgmode/blob/master/lua/orgmode/org/autocompletion/init.lua#L23

Is this for omnicomplete or other aspects? I built out a selection UI that looks similar to the one I've seen commonly used in Emacs with org-roam. Plan to keep this UI for node completion at point and other selections, but if you have something built in that handles providing omni completion, I'd definitely want to supply ids from my database to it.

Use orgmode capture class with custom templates only for org-roam where it would append the properties with id

I haven't looked at this yet. I know that Emacs' org-roam implementation needed to explicitly create its own templating system due to some incompatibilities with orgmode's templates; however, I don't know what those are and I would much rather use what you've already built.

In the issue description you wrote you are using commit 651078a. That's a fairly old one, and id was not supported there yet. To generate ids you can use this class https://github.com/nvim-orgmode/orgmode/blob/master/lua/orgmode/org/id.lua

Good to know. I've got different versions on different machines. Given I started fiddling with this a couple of months back, I guess the org id was implemented after.

Not sure if this information changes anything for you, but I planned to do this. I generally wouldn't suggest using internal classes, but I planned to do that since everything would be part of the same organization.

I want to reduce the hacks and redundancy where possible. My implementation is meant to build on top of your plugin to supply the org-roam features, but when I started it looked like there was enough missing that I ended up only leveraging the treesitter language tree to get the information I needed.

Would be interested in working with you on bridging the gap in functionality and getting this plugin moving forward, unless you were wanting to build your own version of org-roam.

@kristijanhusak
Copy link
Member

We can either have a separate issue or rename this one.

Haven't looked at this. I wanted to be fairly flexible in the API used to load files, and I also needed some information I didn't see a couple of months back such as top-level drawer access, location information regarding where links/headings/properties are, detection of links/nodes under the cursor, etc.

You can check this folder for all the logic around loading files and accessing different elements in the org file https://github.com/nvim-orgmode/orgmode/tree/master/lua/orgmode/files
Files are loaded asynchronously using luv.
Most methods in file and headline provide the element content and the node containing location information (range).
For example, I recently added get_properties() and get_property(name) to be able to get top-level properties from a file. These do not contain any information about the location, but we can extend those as we go.

Populating the database, on the other hand, could definitely switch from a custom parser to what you've got, but to my understanding your links and other structures do not capture their location within a file, which I need in order to show where backlinks, citations, and unlinked references come from.

This method just gets all the links, and creates a Link class, from which you can get different parts of the url. It does not contain a location within the file, but we could add it if you think it will be helpful. I didn't look into the details what you have done, so I'll leave this decision to you.

I haven't looked at this yet. I know that Emacs' org-roam implementation needed to explicitly create its own templating system due to some incompatibilities with orgmode's templates; however, I don't know what those are and I would much rather use what you've already built.

I think you will be able to use it. You just need to create custom Templates class that gives the list of templates, and you can also create a custom OrgRoamTemplate class while extending Template class here, for stuff like dynamic file name with title, slug and such.

Is this for omnicomplete or other aspects? I built out a selection UI that looks similar to the one I've seen commonly used in Emacs with org-roam. Plan to keep this UI for node completion at point and other selections, but if you have something built in that handles providing omni completion, I'd definitely want to supply ids from my database to it.

Yes this is for omnicompletion and completion through cmp, basically any autocompletion. You can see how other builtin sources are added and you can add your own through add_source method.

Would be interested in working with you on bridging the gap in functionality and getting this plugin moving forward, unless you were wanting to build your own version of org-roam.

I wanted to build my own version that would be part of https://github.com/nvim-orgmode organization, but I will not have time to do that any time soon (like 6 months), so it's probably best to delegate this to you since you already did a lot of stuff for it.

@chipsenkbeil chipsenkbeil changed the title Support folding in a buffer without a file [plugin] org-roam.nvim Mar 16, 2024
@chipsenkbeil
Copy link
Contributor Author

We can either have a separate issue or rename this one.

Renamed this one.

You can check this folder for all the logic around loading files and accessing different elements in the org file https://github.com/nvim-orgmode/orgmode/tree/master/lua/orgmode/files Files are loaded asynchronously using luv. Most methods in file and headline provide the element content and the node containing location information (range). For example, I recently added get_properties() and get_property(name) to be able to get top-level properties from a file. These do not contain any information about the location, but we can extend those as we go.

I'll give these a look in a week or two to see how they operate.

This method just gets all the links, and creates a Link class, from which you can get different parts of the url. It does not contain a location within the file, but we could add it if you think it will be helpful. I didn't look into the details what you have done, so I'll leave this decision to you.

I don't know if it makes sense to add it to that method, but the locations of links within a file are needed for org-roam in order to support listing multiple references to the same backlink and to be able to pull in a sample of the content that linked to a node. I think it's easier to see from this person's tutorial of the Emacs usage: https://youtu.be/AyhPmypHDEw?si=mLGsAdosnKjTZ-1f&t=1690

I think you will be able to use it. You just need to create custom Templates class that gives the list of templates, and you can also create a custom OrgRoamTemplate class while extending Template class here, for stuff like dynamic file name with title, slug and such.

I was really hoping that I could reuse it, so this makes me happy to hear. :) Will be giving that a look as soon as I reach that point.

Yes this is for omnicompletion and completion through cmp, basically any autocompletion. You can see how other builtin sources are added and you can add your own through add_source method.

Got it. Yes, I definitely want to use your code to handle omnicompletion. The selector is more like a built-in telescope interface, which I wanted to have similar to what is shown in the emacs tutorial above when it comes to selecting nodes where you can not only select between choices but also filter the choices further. So having both is my plan.

I wanted to build my own version that would be part of https://github.com/nvim-orgmode organization, but I will not have time to do that any time soon (like 6 months), so it's probably best to delegate this to you since you already did a lot of stuff for it.

I don't really mind where my plugin lives, so if you would be interested in taking this in at some point in the future once we remove as much of the custom logic as makes sense, then I'd be happy to hand it over to you. I need something like this for work, and it didn't exist, which is why I'm building it now.

@kristijanhusak
Copy link
Member

I added range property to links in c4eeb3d that holds the position of the links, including [[ and ]] markers.

We can add a few more things as we go if you find them missing, just let me know.

@chipsenkbeil
Copy link
Contributor Author

@kristijanhusak are these indexed starting at 0 or 1?

@kristijanhusak
Copy link
Member

It's 1.

@chipsenkbeil chipsenkbeil changed the title [plugin] org-roam.nvim [plugin] org-roam.nvim progress & discussion Mar 16, 2024
@chipsenkbeil
Copy link
Contributor Author

@kristijanhusak I just updated to c4eeb3d and when trying to open an id link using org_open_at_point (bound to a local leader mapping), I'm still getting the issue about "No headline found with id: ...". I'm assuming this is because having a file-level id is unique to org-roam and not orgmode, which I "think" only has the concept of ids in property drawers at the heading level. But I'm not sure if global ids follow that logic or not.

From https://orgmode.org/manual/Handling-Links.html

If the headline has a ‘CUSTOM_ID’ property, store a link to this custom ID. In addition or alternatively, depending on the value of org-id-link-to-org-use-id, create and/or use a globally unique ‘ID’ property for the link 28. So using this command in Org buffers potentially creates two links: a human-readable link from the custom ID, and one that is globally unique and works even if the entry is moved from file to file. The ‘ID’ property can be either a UUID (default) or a timestamp, depending on org-id-method. Later, when inserting the link, you need to decide which one to use.

Given this fact and your desire to maintain only core orgmode functionality within this plugin, I'm assuming that unless the global ID at a file level is part of core orgmode, I will need to write my own open logic for links that navigates to the correct id that supports file-level ids. The thought process is to have that function fall back to your implementation if the link is not an id link. Thoughts?

@chipsenkbeil
Copy link
Contributor Author

I changed the org-roam buffer to more closely match the org-roam variation. Looks like they changed to use magit underneath, which I'm just replicating in style right now.

Also, progress on following node change under cursor. Don't know the performance considerations of this given I've got tiny tests.

cursor-following-buffer.mp4

@kristijanhusak
Copy link
Member

Id links work fine for me. There's also a test that confirms that it's working. Note that your files need to be part of org_agenda_files otherwise, it won't be found.

@chipsenkbeil
Copy link
Contributor Author

@kristijanhusak your test has a property drawer with an id that is within a section with a headline. Does it work with a top-level property drawer?

image

The case I'm referring to is a top-level property drawer not within a section.

image

My dotfiles configure every org file (including those of the roam directory) to be within the agenda:

image

@kristijanhusak
Copy link
Member

Ah yes, you are correct, there was no support for top-level id. I added that now on the latest master, let me know if it's not working as expected.

@chipsenkbeil
Copy link
Contributor Author

@kristijanhusak fantastic! Quick test has it working just fine. :) One less thing I have to manage myself.

@chipsenkbeil
Copy link
Contributor Author

@kristijanhusak I haven't been able to find it yet. Do you have a version of org-id-get-create (part of org-id.el)?

This is referenced in org-roam's manual and is used to inject an ID into a head as seen in this demo.

@kristijanhusak
Copy link
Member

Yes, you can use this https://github.com/nvim-orgmode/orgmode/blob/master/lua/orgmode/org/id.lua

local id = require('orgmode.org.id').new()

There's also a method on headline classid_get_or_create that adds id property to a headline if there isn't one already.

@chipsenkbeil
Copy link
Contributor Author

There's also a method on headline classid_get_or_create that adds id property to a headline if there isn't one already.

This is what I'm specifically looking for, which both does the work of generating the id and placing it in the headline. Is there a version that also works with a property drawer at the file level?

When I tested org-id-get-create in Emacs, it works with a property drawer at the file level. So, ideally, this would be a command that someone could run whether they're in a heading or not.

Here's an example:

org-id-get-create.mp4

@kristijanhusak
Copy link
Member

kristijanhusak commented Mar 17, 2024

We could add that, but I don't think you will need it. For org-roam you will have custom templates that will already populate this information before showing the capture window. When creating a template, just call the orgmode.org.id to generate an id for you.

function OrgRoamTemplate:compile()
   --call parent compile
  local content = OrgTemplate:compile()
  content =  vim.list_extend({
    ':PROPERTIES:',
    ':ID: '..require('orgmode.org.id').new(),
    ':END:'
  }, content)
end

@chipsenkbeil
Copy link
Contributor Author

chipsenkbeil commented Mar 17, 2024

@kristijanhusak I'm switching back to having the org-roam buffer be an actual org file w/ syntax. I've seen examples of org-roam that use magit as the interface and others where it has a plain org file.

From the discussions I've seen regarding how the buffer is used, people will either use it to open a backlink/citation/unlinked reference directly in the frame (via RET) or they can force it to open in a different frame (via C-u RET).

I know OrgMappings:open_at_point() exists to open the link or date at the point given. Is there any function that lets you open at point while specifying a different window? Was looking at that function and considering if I need to build a wrapper for that function to be able to point to a different window.

Org-roam manual reference

Link: https://www.orgroam.com/manual.html#Configuring-the-Org_002droam-buffer-display

Crucially, the window is a regular window (not a side-window), and this allows for predictable navigation:

  • RET navigates to thing-at-point in the current window, replacing the Org-roam buffer.
  • C-u RET navigates to thing-at-point in the other window.

Emacs manual reference

Link: https://orgmode.org/org.html#Handling-Links

There's a minor reference to org-link-frame-setup, which appears to let you configure how files and directories are opened including other frames:

image

There's a minor reference to org-link-use-indirect-buffer-for-internals, which I opened up below:

image

Other references

@kristijanhusak
Copy link
Member

We could extend it to accept the command that opens the "point", and default it to edit. Would that work for you?

@chipsenkbeil
Copy link
Contributor Author

chipsenkbeil commented Mar 17, 2024

We could extend it to accept the command that opens the "point", and default it to edit. Would that work for you?

I think that should work, yeah. As long as there is a way for me to specify the window to use versus the current window.

I think I'd do this by using wincmd:

---@param winnr integer # id of window where we will open buffer
---@return string # to supply to vim.cmd(...)
function make_cmd(winnr)
    local cmd = winnr .. "wincmd w" -- goes to window "winnr"
    return cmd .. " | edit"
end

Update

Leveraging loading a singular orgfile per preview in the org buffer. I was reading through how org-roam does this (via changelog and source), and it looks like it handles certain cases specially such as detecting if the link is in a heading and just showing the heading text or detecting if the link is in a list item and showing the entire list.

I'll be switching this back over to an org buffer soon, but here's a preview:

image

This is my logic for now, although I'm assuming I should create a new OrgFiles once with the path to the org roam directory, versus creating a new instance each time. So I'll be doing that change. Does loading org files check the modification time or something else to return a cached version? In my old approach, I would stat each file to check its mtime.sec against a cached version.

-- NOTE: Loading a file cannot be done within the results of a stat,
--       so we need to schedule followup work.
vim.schedule(function()
    require("orgmode.files")
        :new({ paths = opts.path })
        :load_file(opts.path)
        :next(function(file)
            ---@cast file OrgFile
            -- Figure out where we are located as there are several situations
            -- where we load content differently to preview:
            --
            -- 1. If we are in a list, we return the entire list (list)
            -- 2. If we are in a heading, we return the heading's text (item)
            -- 3. If we are in a paragraph, we return the entire paragraph (paragraph)
            -- 4. If we are in a drawer, we return the entire drawer (drawer)
            -- 5. If we are in a property drawer, we return the entire drawer (property_drawer)
            -- 5. If we are in a table, we return the entire table (table)
            -- 5. Otherwise, just return the line where we are
            local node = file:get_node_at_cursor({ opts.row, opts.col - 1 })
            local container_types = {
                "paragraph", "list", "item", "table", "drawer", "property_drawer",
            }
            while node and not vim.tbl_contains(container_types, node:type()) do
                node = node:parent()

                -- Check if we're actually in a list item and advance up out of paragraph
                if node:type() == "paragraph" and node:parent():type() == "listitem" then
                    node = node:parent()
                end
            end

            -- Load the text and split it by line
            local text = file:get_node_text(node)
            item.lines = vim.split(text, "\n", { plain = true })
            item.queued = false

            -- Schedule a re-render at this point
            opts.emitter:emit(EVENTS.CACHE_UPDATED)
            return file
        end)
end)

@kristijanhusak
Copy link
Member

we could also make the argument a function, that would default to:

local edit_cmd = edit_cmd or function(file) return 'edit '..file end

vim.cmd(edit_cmd(filename))

That might give you more control over it.

I should create a new OrgFiles once with the path to the org roam directory, versus creating a new instance each time. So I'll be doing that change. Does loading org files check the modification time or something else to return a cached version?

Yes, you should go with the single instance. Here I have an Org instance that holds other instances.
Regarding caching, I do same for files that are not loaded, and check the buffer changedtick if the file is loaded inside a buffer. You can see the logic here

function OrgFile:is_modified()
local bufnr = self:bufnr()
if bufnr > -1 then
local cur_changedtick = vim.api.nvim_buf_get_changedtick(bufnr)
return cur_changedtick ~= self.metadata.changedtick
end
local stat = vim.loop.fs_stat(self.filename)
if not stat then
return false
end
return stat.mtime.nsec ~= self.metadata.mtime
end

@chipsenkbeil
Copy link
Contributor Author

we could also make the argument a function, that would default to:

local edit_cmd = edit_cmd or function(file) return 'edit '..file end

vim.cmd(edit_cmd(filename))

That might give you more control over it.

I think either solution should work.

Org roam buffer

I'm still hitting some quirks with creating an org buffer that is of buftype = nofile.

  1. As you pointed out earlier, I have to supply .org at the end of the buffer name. It looks like as a result of this, orgmode updates the buffer name to be relative to the orgfiles directory for me. So now if I want to ensure that I'm not within my own buffer, I have to do vim.endswith(vim.api.nvim_buf_get_name(0), "org-roam-buffer.org") instead of equality.
  2. It looks like my buffer's preferences to be both unlisted and a scratch buffer get overwritten. Maybe you're swapping out the buffer on me (don't know yet). This means that this temporary buffer shows up in the buffer list.
  3. I still can't get folding to work. Running zx doesn't appear to update the folds.
  4. Reloading the buffer clears the contents (this is on me to fix) and requires to jump elsewhere to refresh. But the callout is that the syntax highlighting appears to break partially when the buffers contents are reloaded again. The comment and heading colors.
example-of-org-buffer-issues.mp4

@kristijanhusak
Copy link
Member

I would need to investigate how scratch buffer behaves. Could you try using a temp filename and see if it's better (vim.fn.tempname()..'org')? I'm not sure if that would work, but if you reuse it and write it each time it's changed, it might work.

@chipsenkbeil
Copy link
Contributor Author

chipsenkbeil commented Apr 12, 2024

@kristijanhusak also, even with the changes you've made and the PR I submitted, I'm still having updates of the property fail. It looks like the end line/col may be off. If this is zero-indexed, line 6 is correct for the position of the property. So line 5 is incorrect. The buffer is the right one, though.

image

I'm assuming this is because you're using vim.fn.line and vim.fn.col, which when used with $ return the last line in the current buffer. I suspect I didn't catch this earlier because I had more lines or equivalent lines or something in my selection dialog buffer as I did in the org buffer.

I think you could replace vim.fn.line('$') with vim.api.nvim_buf_line_count(bufnr) to avoid being locked to the current buffer.

Example Fix for OrgFile:set_node_text

For vim.fn.col({ end_row, '$' }), you'd have to do something like string.len(vim.api.nvim_buf_get_lines(bufnr, end_row, end_row + 1, true)[1]) assuming that end_row was zero-indexed.

image image

@chipsenkbeil
Copy link
Contributor Author

chipsenkbeil commented Apr 13, 2024

@kristijanhusak another issue I noticed that is unrelated. It seems like something my plugin is doing is preventing agenda from working. With my plugin installed, accessing the org agenda via <Leader>oa and displaying the schedule or all TODOs, both are blank. When I comment out my plugin, they display TODOs as expected.

I'm not within a roam file, but even if I was I wouldn't expect roam to interfere with agenda information. I do know that when I do something like print(vim.inspect(require("orgmode").agenda)), I can see files are loaded in. I have TODOs outside of my roam directory, although I'd like to be able to have TODOs anywhere.

Is there something I can debug to figure out why this is happening? Any theories regarding what I may have touched or be modifying that causes this issue?m

Found the problem

It looks like the OrgAgenda is using my org-roam directory instead of my orgmode directory.

image

image

Found the underlying problem & solution

Turns out, OrgFiles:new(...) is overwriting the paths field for all instances of OrgFiles since self at this point is OrgFiles and not data.

image

You may want to scan through other cases of construction to see if it's accidentally using self when the intention is the instance. The pattern I typically follow for instance creation looks like this:

---@class SomeClass
---@field some_field string
---@field other_field integer
local M = {}
M.__index = M

---Creates a new instance of SomeClass.
---@param opts {some_field:string}
---@return SomeClass
function M:new(opts)
    local instance = {}
    setmetatable(instance, M)

    -- Populate fields for the instance
    instance.some_field = opts.some_field
    instance.other_field = 1234
    instance:__do_extra_setup()

    return instance
end

@kristijanhusak
Copy link
Member

I pushed fixes for the buf issues you opened PR for, and the fix for the file paths: 5875037

@chipsenkbeil
Copy link
Contributor Author

@kristijanhusak thanks for the speedy resolution on that one 😄

Another quirk I ran into by accident was when I tried to load an org file, but forgot to place the extension .org. It turns out that the method OrgFile.load should have the return signature of OrgPromise<OrgFile | false> versus OrgPromise<OrgFile> and thereby OrgFiles:add_to_paths and OrgFiles:load_file (and maybe others) also need an updated return signature.

I ran into this issue because I thought the promise would only resolve on success, but it also resolved when returning false, which I did not realize that I needed to handle.

@kristijanhusak
Copy link
Member

Added types in ba2ab78

@chipsenkbeil
Copy link
Contributor Author

Added types in ba2ab78

Great! I'm back from vacation, and refactoring the plugin to be more easily testable and been writing test coverage before the release. Nearly there. 😄

@chipsenkbeil
Copy link
Contributor Author

@kristijanhusak one quirk I've noticed for awhile, and am not sure if it's expected, is that directives don't parse if there is text immediately after them on the following line.

In the recording below, I show the title that is calculated for a node. If there is a #+TITLE directive, it is used, otherwise it uses the file name. When I have a newline between the directive and something else, it finds the directive, but if there is anything following the directive, it does not. It's confusing because syntax highlighting for the directive still works. What's the expectation?

example-of-directive-not-working.mp4

@kristijanhusak
Copy link
Member

It happens because once you add some content that is not a directive, it converts it to paragraph node, and nests directive inside. You can do InspectTree and see the difference.
I'm not 100% sure how Emacs treats those.

For example, if you have this:

#+TITLE: File title
Some content
#+AUTHOR: Kristijan

* Some headline

Is AUTHOR still a valid directive for a file or it's only TITLE ?

@chipsenkbeil
Copy link
Contributor Author

chipsenkbeil commented Apr 19, 2024

It happens because once you add some content that is not a directive, it converts it to paragraph node, and nests directive inside. You can do InspectTree and see the difference. I'm not 100% sure how Emacs treats those.

Yeah, that's what I assumed was happening, but the confusing aspect is that the directive is still highlighted like a directive instead of like part of the paragraph, which is why it was surprising that it didn't work earlier. Why is that the case if the treesitter parser doesn't treat it like a directive anymore?

Is AUTHOR still a valid directive for a file or it's only TITLE ?

My expectation would be that the directives work even with content above/below them because Emacs has plenty of examples of this including directives that expect something on the following line. And things like #+INCLUDE would definitely support being placed anywhere in the file.

image

And if I search github, while I'll find plenty of examples that include a newline between a series of directives and the start of content, I'll also find examples where directives have content immediately following them like this one.

While I haven't tested anything specific in Emacs, the highlighting at least alludes to it being a directive.

image

@kristijanhusak
Copy link
Member

but the confusing aspect is that the directive is still highlighted like a directive instead of like part of the paragraph, which is why it was surprising that it didn't work earlier

Highlighting targets (directive) node directly, that's why it works. Finding body level directive goes through (body) node.

Generally, the parser has some issues figuring out when something is a paragraph and when something is a directive on the body level. I tried figuring out what can be the issue but I don't have that much experience writing parsers. If you have any idea please take a look at https://github.com/nvim-orgmode/tree-sitter-org.

@chipsenkbeil
Copy link
Contributor Author

but the confusing aspect is that the directive is still highlighted like a directive instead of like part of the paragraph, which is why it was surprising that it didn't work earlier

Highlighting targets (directive) node directly, that's why it works. Finding body level directive goes through (body) node.

Generally, the parser has some issues figuring out when something is a paragraph and when something is a directive on the body level. I tried figuring out what can be the issue but I don't have that much experience writing parsers. If you have any idea please take a look at https://github.com/nvim-orgmode/tree-sitter-org.

I can take a look, although I think the general solution for me is to query for directives anywhere similar to syntax highlighting, right? Is there a downside to doing that? They can show up anywhere in a document as long as they start the line.

@kristijanhusak
Copy link
Member

You can do that, but that might be misleading since directives can be used for a few other things like tables for example. What do you need a directive for?

@chipsenkbeil
Copy link
Contributor Author

You can do that, but that might be misleading since directives can be used for a few other things like tables for example. What do you need a directive for?

At the moment, simple things: title and file tags. From what I can see of the org roam manual, nothing else in v2 (the version I'm mirroring and extending) uses directives. I'm more thinking on the easiest way to find #+TITLE and #+FILETAGS if they happen to get lumped into a different element.

image

@chipsenkbeil
Copy link
Contributor Author

@kristijanhusak I ran into a weird bug. Haven't been able to reproduce it yet.

I was in the directory interface of lir.nvim, deleted all of the files within, and then tried to open a capture buffer via the org roam api. The below error popped up, and after that I was presented with the template selection dialog like usual.

image

@chipsenkbeil
Copy link
Contributor Author

chipsenkbeil commented Apr 27, 2024

@kristijanhusak noticed another missing return, which messes with the language server a bit. From orgmode.objects.calendar, the call to Calendar.new doesn't have a return. This causes me issues with chaining via Calendar.new():open(). Wait, no, the issue is that data isn't marked as optional, so I have to do Calendar.new({}):open() to satisfy the language server, even though you put that in as a default.

image

Highlighting calendar dates

Working on the dailies extension of org roam, which should be a quick addition. Only thing I'm not sure about is if there's a way to specify in the calendar interface which days to highlight a certain color. With org roam, it does some injection to highlight days in a month that have a pre-existing note with a blue color versus the default black text. Any thoughts on how to do that with your calendar api?

@chipsenkbeil
Copy link
Contributor Author

@kristijanhusak I know we've made changes to the capture API earlier. Is there any chance that one additional callback/handler could be supported that reports when a refile is canceled?

I'm going through the org roam plugin and rewriting functions that use callbacks to instead return promises. The issue is that there's no way for me to fully resolve a promise that I'm creating tied to a capture, where I return the newly-created node's ID or nil if canceled because I have no callback when the capture is canceled.

Today, I'm using the pre_refile and post_refile callbacks for material we discussed. If there was a way to supply an on_cancel callback, then I'd be able to have a fully-resolvable promise wrapper around capturing an org roam node.

@kristijanhusak
Copy link
Member

Added on_cancel_refile in d4cc321

@chipsenkbeil
Copy link
Contributor Author

Highlighting calendar dates

Working on the dailies extension of org roam, which should be a quick addition. Only thing I'm not sure about is if there's a way to specify in the calendar interface which days to highlight a certain color. With org roam, it does some injection to highlight days in a month that have a pre-existing note with a blue color versus the default black text. Any thoughts on how to do that with your calendar api?

@kristijanhusak thoughts on this one?

@kristijanhusak
Copy link
Member

@chipsenkbeil are you opening the calendar yourself or using some of the existing orgmode mappings?

@chipsenkbeil
Copy link
Contributor Author

@chipsenkbeil are you opening the calendar yourself or using some of the existing orgmode mappings?

I'm invoking Calendar.new():open() at the moment. I have logic to load up all of the populated dates, but not using it for anything right now.

https://github.com/chipsenkbeil/org-roam.nvim/pull/34/files#diff-958febaccfe0cbf1036a3e0de6000e347a6e4eb25f95ee942f3443945287c5a9R224-R243

@kristijanhusak
Copy link
Member

This is how today's date is highlighted
https://github.com/nvim-orgmode/orgmode/blob/master/lua/orgmode/objects/calendar.lua#L190-L200

We can add a hook like on_render(OrgDate) and you could apply similar logic.
Another solution is to have a hook for each day that is triggered and then have a calendar handle if it should highlight it or not and how. That would be a much bigger work, which I'm not sure when I'll be able to get to.

@chipsenkbeil
Copy link
Contributor Author

This is how today's date is highlighted https://github.com/nvim-orgmode/orgmode/blob/master/lua/orgmode/objects/calendar.lua#L190-L200

We can add a hook like on_render(OrgDate) and you could apply similar logic. Another solution is to have a hook for each day that is triggered and then have a calendar handle if it should highlight it or not and how. That would be a much bigger work, which I'm not sure when I'll be able to get to.

How would on_render work? Do I return a highlight group? A boolean? The logic you showed scans through each line and applies highlights. Are you thinking that on_render is called and I'll need to do the same date scan?

@chipsenkbeil
Copy link
Contributor Author

chipsenkbeil commented May 5, 2024

@kristijanhusak heads up, Promise.wait bit me recently. Or, rather, my own assumption got me.

I assumed that Promise.wait would wait up to the timeout period, and throw an error if the timeout was reached without a resolution. Instead, nil is returned, which still will cause failures (unless you expect nil as a return value), but less clear. The type defined for the generic function indicates that wait will always return V.

I submitted a PR to update Promise.wait to fail versus silently succeeding on timeout: #723

@chipsenkbeil
Copy link
Contributor Author

chipsenkbeil commented May 7, 2024

This is how today's date is highlighted https://github.com/nvim-orgmode/orgmode/blob/master/lua/orgmode/objects/calendar.lua#L190-L200

We can add a hook like on_render(OrgDate) and you could apply similar logic. Another solution is to have a hook for each day that is triggered and then have a calendar handle if it should highlight it or not and how. That would be a much bigger work, which I'm not sure when I'll be able to get to.

A thought I had for how to do this - but I want to get your perspective first - is to have a loop through all lines that uses a similar matching to your today check. The user of the calendar would register something like on_render_day(OrgDate):string|nil that would be invoked on each day, returning a string representing the highlight to apply. Or maybe give even more control by having a signature like on_render_day(date:OrgDate, data:{buf:integer, ns:integer, line:integer, col_start:integer, col_end:integer}) where you could apply a highlight yourself.

Just after the check for highlighting the current day, I could see this code within render:

for i, line in ipairs(content) do
    local from = 0
    local to, num

    -- Process all days on the line
    while true do
        from, to, num = line:find("%s(%d%d?)%s", from + 1)
        if from == nil then break end
        if from and to then
            local date = Date:new({ year = self.month.year, month = self.month.month, day = num })
            local hl = self.on_render_day(date)

            -- If the user-provided function returns a highlight group, we apply it
            -- Alternatively, the on_render_day could take the buffer, namespace, line, col_start, col_end to do its own thing
            if hl then
                vim.api.nvim_buf_add_highlight(self.buf, namespace, hl, i - 1, from - 1, to)
            end
        end
    end
end

This could potentially replace the current logic to highlight today, and just have that be the default render day function.

@kristijanhusak
Copy link
Member

I had a slightly different idea, but this one also works. I'll refactor it later if necessary.
Added on_day hook for calendar. You can see how it's implemented here cda615f.
For example, to highlight weekend days you would do something like this:

  return Calendar.new({
    date = Date.now(),
    on_day = function(day, opts)
      if day:is_weekend() then
        vim.api.nvim_buf_add_highlight(
          opts.buf,
          opts.namespace,
          'WeekendHLGroup',
          opts.line - 1,
          opts.from - 1,
          opts.to
        )
      end
    end,
  })

@chipsenkbeil
Copy link
Contributor Author

@kristijanhusak works like a charm!

image

@chipsenkbeil
Copy link
Contributor Author

chipsenkbeil commented May 11, 2024

@kristijanhusak I can't think of anything else I need for the core roam functionality other than #718, which I know is a larger ask than you have time for at the moment. 😄

So with that being said, thoughts on closing this issue and #66? If possible, I'd love to have a newly-tagged version of the orgmode plugin, so I can update my README instructions to reference needing orgmode 0.3.x+ or 0.4+ instead of a specific commit.

I've got a video in the works to run through the plugin, so will be excited to share that in the future! Thanks again for all of the support you've given me over the past two months!

@kristijanhusak
Copy link
Member

Created release https://github.com/nvim-orgmode/orgmode/releases/tag/0.3.4.
If you run into something feel free to open up a separate issue.
Thanks for working on the plugin, it will be very welcomed by the community! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants