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

Add --linked-attachments-only option #316

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

jajaperson
Copy link

@jajaperson jajaperson commented Dec 7, 2024

Adds support for #217 via a --linked-attachments-only flag. When enabled, any non-markdown files will only be included if they are linked or embedded within a markdown file.

@jajaperson
Copy link
Author

I've just worked out that this approach doesn't work with the tag filtering options, I will try fix this now.

Make `linked_attachments_only` option compatible with tag filtering,
i.e. only occur after post-processing.
Copy link
Owner

@zoni zoni left a comment

Choose a reason for hiding this comment

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

Thank you so much for working on this. I've yet to do a more thorough review of the code, but at a glance, I think this looks alright, so I'm favorable towards accepting it 😄

However, I would like to see some tests being included to validate the intended behavior (including the interaction with tag filtering options) and guard against future regressions as outlined in Contributing > Tests.

Would you mind wiring up some test cases? I'll be happy to provide some additional pointers if needed.

@jajaperson
Copy link
Author

Sure, I'll be able to write some tests in a few days. By the way, the way it's currently implemented, any attachments detected during markdown parsing are added to a HashSet. Once parsing is complete for that file, the export_note method is called for each of its attachments. So potentially, the same attachment is getting exported more than once, if it is linked in multiple markdown files. Could this, to your knowledge, cause any issues? Ideally there would be a centralized HashSet collated over all markdown files which is then iterated over at the end, but I didn't come up with an easy solution for this with the way the Exporter is currently setup with parallelism.

@jajaperson jajaperson requested a review from zoni December 14, 2024 08:01
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