feat: option to inject specific metadata instead of defaults #1128

Dynge · 2023-10-25T12:35:40Z

This PR closes #1126. I did not want to deprecate any existing functionality so I've made new functions to write_metadata with potential new values instead of defaults.

Challenges

I am unsure if there is still is any need to use fs_open() to create the file, now that I instead create a new tab if no_open is true. My reasoning for creating a new tab is that in order to inject_metadata i need to have a buffer number, so I always open the new file in a new buffer. With tabs i can easily close the new tab without needing to worry about what else was open at the time.

Additional notes

I noticed that there does not seem to be an easy way to add categories into the metadata as it seems the metadata fields need to return a single string.
I thought about added functionality to allow tables of strings as well such that you could have a method return a collection of categories (e.g. { "[", " cat1", " cat2", "]" })

This method can be used to add custom metadata into file instead of defaults. If table is empty of nil then simply uses default like inject_metadata

max397574 · 2023-10-25T12:51:29Z

Tbh I don't like the way you do this. The whole things with the vim commands (:e, :tabnew etc)
Metagen just needs a bufnr and doesn't require the file to be open.
There are api functions to load a file to get a usable bufnr. I could send you those later.

max397574 · 2023-10-25T13:01:32Z

Here is an example where this was done
nvim-neorg/neorg-telescope@34a41e8#diff-38d33560722aa6a06ec8b1cd99066184484042c5302b0fdf0718d0afad1c7029R72-R93
not sure if you need to load the buffer or not for metagen. If not you can just use the function from dirman which's used there
or see how it's done there and just inline it

edit: realized you are in the dirman module so it should be really easy to use the function

Dynge · 2023-10-25T14:22:13Z

Ah perfect. Thanks for the pointers @max397574

Ill edit the file to use that instead.

Any other thoughts on the function names and usage?

Also is there somewhere else i should write dokumentation for the usage?

max397574 · 2023-10-25T14:55:14Z

you can just write docs right above the function and inside the comment at the top of the file

No longer uses tabnew and relies on buffer 0 to insert metadata into the newly created file.

Dynge · 2023-10-25T18:16:02Z

I've changed the code to use dirman.get_file_bufnr instead - making it much cleaner.

Also tried to add some more documentation to describe the change.

Seems to work great - the only thing I am a little unhappy about is the implementation of categories that does not work so great in this.

It would be nice if categories could be a table and then it would insert them correctly on separate lines - like this:

dirman.create_file("note", "notes", { metadata = { categories = { "work", "programming" } } })

However that does not work in the current implementation because metadata is expected to be simply a string or a function that returns a string - not a table.
Its a special case because categories have a special syntax rule for multiline.
This may expand the scope of this pull request - unless there exists a very easy fix for that, that i am not aware of :)

max397574 · 2023-10-25T18:22:42Z

lua/neorg/modules/core/dirman/module.lua

+dirman.create_file("my_file", "my_ws", {
+    no_open  = false,  -- open file after creation?
+    force    = false,  -- overwrite file if exists
+    metadata = {}      -- key-value table for metadata fields


what happens when you pass an empty table there? will just default metadata be generated?

Yes.
It overrides default if there's a match in the key names. Otherwise it does nothing.

max397574 · 2023-10-25T18:24:00Z

lua/neorg/modules/core/dirman/module.lua

@@ -271,6 +284,9 @@ module.public = {
    ---@param opts? table additional options


normally people these days use classes for annotations in cases like this
e.g.

---@class neorg.dirman.new_note_opts ---@field no_open bool

Not aware of how to do that.
Can you point me to an example?

I see this
https://github.com/LuaLS/lua-language-server/wiki/Annotations#class

Should i just create an empty unused table in the top of the file and annotate that?

Is that the way to do it? 😄

Im quite new to lua 😅

you can just write the "class comments" as in my example above somewhere above the function. It would just be a comment block without any "real" code where you define the class with the fields (see this example https://github.com/LuaLS/lua-language-server/wiki/Annotations#field). And then you just use that class as the type of the parameter opts.

@max397574 I added @Class documentation now :)

max397574 · 2023-10-25T18:26:07Z

haven't tested the functionality but code lgtm

Dynge · 2023-10-26T05:28:56Z

Just realize there may be a bug.

I do not think its possible to create a file without metadata if metagen is installed anymore.

We need to check if theres auto metagen setting enabled as well. However om not sure how it should work if auto metagen is off and you provide a non-empty table...

Any thoughts?

max397574 · 2023-10-26T05:46:02Z

I think if there is a table it should add metadata. Just set the default to nil and I think it should work as expected.

Dynge · 2023-10-26T10:16:49Z

Alright. Ill test it out and later today 👍

Added class annotations to better describe the input options for create_file and create_metadata.

spaces instead of tabs

max397574 · 2023-10-26T12:18:10Z

lua/neorg/modules/core/dirman/module.lua

@@ -265,12 +278,16 @@ module.public = {
            })
        end)
    end,
+
+    ---@class create_file_opts


classes are kinda global so you might want to use e.g. neorg.create_file_opts afaict this is common-practice.

Should it go even deeper then and name it neorg.dirman.create_file_opts then?

up to you
you could search through neorg rq (just telescope live grep for ---@class (perhaps with space after ---)) and see how it's done in other parts of the codebase

Updated names to match the apparent naming convention used in the rest of the codebase (as far as i can tell :))

Dynge · 2023-10-26T12:20:09Z

I think if there is a table it should add metadata. Just set the default to nil and I think it should work as expected.

Tested it with core.esupports.type = "none" and it works (it does not add metadata unless table is not nil) :)

@Class

Updated naming of @Class annotations to better align with the naming used in the rest of the project.

vhyrro · 2023-11-03T10:08:06Z

lua/neorg/modules/core/dirman/module.lua

+        local metagen = neorg.modules.get_module("core.esupports.metagen")
+        if opts.metadata and metagen then
+            local bufnr = module.public.get_file_bufnr(fname)
+            metagen.write_metadata(bufnr, true, opts.metadata)
        end


All of the code looks good to me apart from this bit. This intertwines the dirman and metagen modules more than it should and doesn't extend to other modules which might also want to do something special when a file is created. Maybe it would be best to create a core.dirman.file_created event whose content is all of the parameters ({ metadata, force, no_open })? That way any module (including metagen) can listen for this event and do the metadata generation on its own without the two modules ever knowing about each other :)

If you're unsure about how to go around implementing it, feel free to ask!

Thanks for the detailed reply. That sounds like a good solution. I am a little unsure how to implement an event.

I would appreciate any help you can give. 😄

Dirman now broadcasts an event on file creation that metagen listen to. Metagen then injects desired metadata if present.

Dynge · 2023-11-07T11:57:05Z

Ive moved metagen creation into something that is executed on an event upon file creation instead.
This way dirman does not know what metagen decides to do with the data and it is properly separated. (Thanks for the tip @vhyrro)

I've also removed duplicated functions that I made previously in this pull request because i did not know that you can call functions just with without all input values :)

I've tested that i can create notes with metagen creation set to "auto" and "never" and it works as expected.

vhyrro · 2023-11-08T14:03:18Z

Ah I'm super sorry for not providing any help. Always ping me a few times so you can be sure I got the message haha.

Now the code looks good to me. Thank you for all the work put into this PR! 💜

…org#1128) closes nvim-neorg#1126

Dynge added 3 commits October 25, 2023 14:21

✨ added option to incluce custom metadata into new file

113176b

✨ added create_metadata to override defaults

de38773

✨ added write_metadata

13d8426

This method can be used to add custom metadata into file instead of defaults. If table is empty of nil then simply uses default like inject_metadata

Dynge requested review from vhyrro, mrossinek and danymat as code owners October 25, 2023 12:35

Dynge added 4 commits October 25, 2023 18:52

🎨 simplified creation of file

ab1c93b

No longer uses tabnew and relies on buffer 0 to insert metadata into the newly created file.

🎨 converted tabs to spaces to adhere to formatting

64b7824

docs(dirman): 📝 extended docs for file creation

21fc667

docs(metagen): 📝 extended docs for metadata creation

0ac07f2

max397574 reviewed Oct 25, 2023

View reviewed changes

max397574 approved these changes Oct 25, 2023

View reviewed changes

Dynge added 2 commits October 26, 2023 13:46

docs(metadata): 📝 class annotations

196c2e9

Added class annotations to better describe the input options for create_file and create_metadata.

chore(formatting): 🎨 applied proper formatting

eac464a

spaces instead of tabs

max397574 reviewed Oct 26, 2023

View reviewed changes

chore(names): 💬 updated names for @classes

680b909

Updated naming of @Class annotations to better align with the naming used in the rest of the project.

vhyrro reviewed Nov 3, 2023

View reviewed changes

refactor(event): ♻️ event on file creation

214db65

Dirman now broadcasts an event on file creation that metagen listen to. Metagen then injects desired metadata if present.

chore(format): fixed formatting

cb81fb1

vhyrro changed the title ~~Option to inject specific metadata instead of defaults~~ feat: option to inject specific metadata instead of defaults Nov 8, 2023

vhyrro merged commit 5509079 into nvim-neorg:main Nov 8, 2023
1 check passed

github-actions bot mentioned this pull request Nov 8, 2023

chore(main): release 6.2.0 #1140

Merged

benlubas pushed a commit to benlubas/neorg that referenced this pull request Jan 11, 2024

feat: option to inject specific metadata instead of defaults (nvim-ne…

978af5d

…org#1128) closes nvim-neorg#1126

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: option to inject specific metadata instead of defaults #1128

feat: option to inject specific metadata instead of defaults #1128

Dynge commented Oct 25, 2023 •

edited

Loading

max397574 commented Oct 25, 2023

max397574 commented Oct 25, 2023 •

edited

Loading

Dynge commented Oct 25, 2023

max397574 commented Oct 25, 2023

Dynge commented Oct 25, 2023

max397574 Oct 25, 2023

Dynge Oct 25, 2023

max397574 Oct 25, 2023

Dynge Oct 25, 2023

Dynge Oct 25, 2023

max397574 Oct 26, 2023

Dynge Oct 26, 2023

max397574 commented Oct 25, 2023

Dynge commented Oct 26, 2023

max397574 commented Oct 26, 2023

Dynge commented Oct 26, 2023

max397574 Oct 26, 2023

Dynge Oct 26, 2023

max397574 Oct 26, 2023

Dynge Oct 26, 2023

Dynge commented Oct 26, 2023

vhyrro Nov 3, 2023

Dynge Nov 3, 2023

Dynge commented Nov 7, 2023

vhyrro commented Nov 8, 2023

		@@ -271,6 +284,9 @@ module.public = {
		---@param opts? table additional options

feat: option to inject specific metadata instead of defaults #1128

feat: option to inject specific metadata instead of defaults #1128

Conversation

Dynge commented Oct 25, 2023 • edited Loading

Challenges

Additional notes

max397574 commented Oct 25, 2023

max397574 commented Oct 25, 2023 • edited Loading

Dynge commented Oct 25, 2023

max397574 commented Oct 25, 2023

Dynge commented Oct 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

max397574 commented Oct 25, 2023

Dynge commented Oct 26, 2023

max397574 commented Oct 26, 2023

Dynge commented Oct 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dynge commented Oct 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dynge commented Nov 7, 2023

vhyrro commented Nov 8, 2023

Dynge commented Oct 25, 2023 •

edited

Loading

max397574 commented Oct 25, 2023 •

edited

Loading