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

fix(summary)!: fix norg links, use first heading as title if found #928

Merged
merged 4 commits into from
Jun 5, 2023

Conversation

laher
Copy link
Contributor

@laher laher commented Jun 3, 2023

Fixes and improvements following on from #927 . I noticed a bug in the link destinations, so I went ahead and did some of what @vhyrro suggested.

  • Fix norg links ( {:index.norg:}[My Index] => {:index:}[My Index] ) - lol, funny I didn't notice this last time.
  • Links now use paths relative to the workspace root. {:index:} rather than {:/Users/me/notes/work/index:} . (It assumes you're in the workspace root, which seems OK?)
  • When there's no metadata.title, it queries TS to find the first heading text. The TS query allows heading1 or heading2, and it checks matches up to 20 lines into the doc (e.g. if there's metadata but no title). IDK if this is right or if the query could be improved, but it seems like an OK starting point.
  • When there's no metadata or headings (e.g. an empty file), then the title becomes Index rather than Index.norg.
  • The summary now includes files which have no metadata (because we can present something useful now).
  • Breaking change - rename the strategy metadata to default because now it uses a mixture of metadata and headings/filenames now.

My summary looks like this now (as before, I only have scraps of document metadata, and some files are completely empty):

** My Heading Which My Cursor Was On
*** Homebase
   - {:index:}[Work Notes]
*** Uncategorised
   - {:help:}[Help]
   - {:inbox:}[Inbox]
   - {:main:}[Neorg Notes]
   - {:neorg:}[Neorg Adventures]
   - {:nostuff:}[Nostuff]
   - {:projects:}[Projects - Medium Term]
   - {:journal/index:}[2023]
   - {:journal/template:}[Template]
   - {:journal/2023/05/04:}[04]
   - {:journal/2023/05/05:}[05 Tues]
   - {:journal/2023/05/06:}[6]
   - {:journal/2023/05/07:}[7]
   - {:journal/2023/05/08:}[8]
   - {:journal/2023/05/18:}[18]

@vhyrro
Copy link
Member

vhyrro commented Jun 4, 2023

Hell yeah. About the workspace root - you can opt for the {:$/index:} syntax, which guarantees that the file exists from the current workspace's root.
It's an extra character but it'll ensure that in case you create a summary from a file that isn't index.norg that everything will still point to the right place!

@vhyrro
Copy link
Member

vhyrro commented Jun 4, 2023

Sorry if you see a million "starting review" notifications i'm playing around with how octo.nvim does things :p

heading_query = neorg.utils.ts_parse_query("norg", heading_query_string)
end
-- search up to 20 lines (a doc could potentially have metadata without metadata.title)
for _, heading in heading_query:iter_captures(document_root, bufnr, 1, 20) do
Copy link
Member

Choose a reason for hiding this comment

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

Check between lines 1 and 20 is alright, but I think we might be better off altogether by just checking for the first occurrence in the file? I can imagine some niche scenarios where you have a long introductory paragraph to a document and only then do you have the title in a first level heading >20 lines down.

If you agree this is the way to go then the simplest way is to ditch the for loop altogether and invoke the function manually to get the first result like this:

Suggested change
for _, heading in heading_query:iter_captures(document_root, bufnr, 1, 20) do
local _, heading = heading_query:iter_captures(document_root, bufnr)()
if not heading then
-- do something
end
-- continue using the `heading` node normally

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done - thanks for the tip, I didn't know about using (..)() to iterate over the first item only.

@vhyrro
Copy link
Member

vhyrro commented Jun 4, 2023

Resolved a conflict w.r.t the latest main commit through the github UI 👍

@laher
Copy link
Contributor Author

laher commented Jun 4, 2023

Hell yeah. About the workspace root - you can opt for the {:$/index:} syntax

Done, ta

@vhyrro
Copy link
Member

vhyrro commented Jun 5, 2023

Just tested and all seems to work great. Thanks a lot for the contributions!

@vhyrro vhyrro changed the title fix(core.summary): fix norg links; use first heading as title if found fix(summary)!: fix norg links, use first heading as title if found Jun 5, 2023
@vhyrro vhyrro merged commit 6f893a2 into nvim-neorg:main Jun 5, 2023
@laher laher deleted the summary-default branch June 5, 2023 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants