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: delimit tangled code blocks with headings #981

Merged
merged 12 commits into from
Aug 4, 2023
Merged

Conversation

jonathf
Copy link
Contributor

@jonathf jonathf commented Jul 6, 2023

An extention to the idea discussed in #955 .

Add heading text as delimiter as a comment if:

  1. There is in-fact a current heading. Headings are optional.
  2. Current heading hasn't been used as a delimiter before.
  3. commentstring is available for the filetype in question.

Otherwise just use a newline.

That last one (3) requires making a temporary scratch buffer to trigger ftplugin, which retrieves commentstring for all supported filetypes in vim.

@rileyshahar
Copy link
Contributor

It would be nice if this supported immediately nested headings, i.e.

* Foo
** Bar
    @code py
    print("hi")
    @end

should output comments with both foo and bar, which I don't think the current impl does.

Also, I think we want to insert the heading even if nothing's been tangled to the current file, whereas we only want to insert a newline when something has been tangled to the current file. If I'm reading it right, the current PR doesn't do that.

@jonathf
Copy link
Contributor Author

jonathf commented Jul 7, 2023

Heading to the beginning of the file is trivial and has been added now.

Immediat nested heading is doable, but I think I want the reviewers input before making the PR more complex than it already is.

@vhyrro
Copy link
Member

vhyrro commented Aug 1, 2023

I like the idea of this! But yeah this should definitely be put behind some feature flag as to not confuse users. It could be interesting as a fancy default, but there also should be a way to turn it off if wanted. I guess we want to read data from the document metadata where we could have some info regarding this. Maybe tangle-delimiter: none|newline|headings or something of the like? :D

@vhyrro
Copy link
Member

vhyrro commented Aug 1, 2023

Additionally I see that there are conflicts here. When I am again free to work solely on Neorg I'll gladly help out with this PR. It might take some time for me to revisit this though! Not ignoring the PR, just busy with priorities :)

@rileyshahar
Copy link
Contributor

@vhyrro any chance we can merge the extremely simple #958 in the interim as a minimal impl of this?

@vhyrro
Copy link
Member

vhyrro commented Aug 1, 2023

sounds good to me!

An extention to the idea discussed in nvim-neorg#958.

Add heading text as delimiter as a comment if:

1. There is in-fact a current heading. Headings are optional.
2. Current heading hasn't been used as a delimiter before.
3. `commentstring` is available for the filetype in question.

Otherwise just use a newline.

That last one (3) requires making a temporary scratch buffer to trigger
ftplugin, which retrieves `commentstring` for all supported filetypes in
vim.
Heading as comment followed by an extra newline above the first block
above the first codeblock assuming it is under a heading. Nothing
otherwise.

As before, an extra newline above the heading as a comment after that.

This give the flow:
```
-- Heading 1

print("Codeblock 1")

-- Heading 2

print("Codeblock 2")
```
@jonathf
Copy link
Contributor Author

jonathf commented Aug 2, 2023

@vhyrro, I am glad you like the idea!

I've done as you suggested and added a configuration option, and moved the branch to the tip of main.

In addition I've done two more things:

  1. Added content to the filetype detection. This mostly means that shebangs now are considered aswell.
  2. Added a generic _ filetype as a catch-all file output when also setting other parameters. This means that tangle: output is equivalent to tangle: {languages: {_: output}}.

I appologies if this is considered feature creep, but I feel it adds to the completness of the feature I am proposing.

Comment on lines -211 to -213
elseif type(parsed_document_metadata.tangle) == "string" then
options.languages[vim.filetype.match({ filename = parsed_document_metadata.tangle })] =
parsed_document_metadata.tangle
Copy link
Member

Choose a reason for hiding this comment

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

I realized that one feature stopped working as a result of this being removed - you should also be able to specify e.g. tangle: ./init.lua in the metadata, which now errors with the new version. It always assumes that languages is a thing, which it may not be :p

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I have a handler for it, on line 220, but it was implemented incorrectly. Fixed now.

@vhyrro
Copy link
Member

vhyrro commented Aug 4, 2023

Tried stress testing it and it seems to work with the cases I threw at it, thank you for this PR! 💜

@vhyrro vhyrro changed the title feat: Delimit tangled code blocks with headings feat: delimit tangled code blocks with headings Aug 4, 2023
@vhyrro vhyrro merged commit 99bfcb1 into nvim-neorg:main Aug 4, 2023
1 of 2 checks passed
benlubas pushed a commit to benlubas/neorg that referenced this pull request Jan 11, 2024
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.

3 participants